core: dep-graph creates invalid dependencies

[Hotell]

Current Behavior

if project A includes static import or require statement to another workspace project within a string (lets say project B), nx will incorrectly resolve this as a dependency on that project, which leads to incorrect project graph creation

• thus running unnecessary packages on CI
• and also giving miss leading output when running nx graph.

UPDATE:

🎬 I was able to narrow down the AST parsing issue and the culprit is template string! whenever there is one being used and followed by string that includes static import then we run into wrong dependency tree resolution.

// @filename packages/proj-a/index.js

const npmScope = 'myorg';
// 🚨 this is the root cause - interpolating template literals 
console.log(`@${npmScope}`);


// 🚨 now your project depends on proj-b ( which is not true )
console.log(`import {foo} from '@my-org/proj-b'`)

following will properly resolve the AST

// @filename packages/proj-a/index.js

const npmScope = 'myorg';
// ✅ no template strings, AST works
console.log('@' + npmScope);


// ✅ this will do no harm as expected
console.log(`import {foo} from '@my-org/proj-b'`)

Expected Behavior

dep-graph creation should ignore static import/require statements included as strings during AST parsing.

Steps to Reproduce

NOTE: download the project to try it out without hassle
ast-issue-dep-graph.tar.gz

1. remove cache `rm -rf node_modules/.cache/nx
2. create 2 projects that contain following:

// @filename packages/proj-b/index.js

export const foo = 'hello'

// @filename packages/proj-a/index.js

const npmScope = 'myorg';
// 🚨 this is the root cause - interpolating template literals 
console.log(`@${npmScope}`);

console.log(`import {foo} from '@my-org/proj-b'`);

export const moo = 'world'

3. run following script:

// @filename repro.js
const { readCachedProjectGraph, createProjectGraphAsync } = require('@nrwl/workspace/src/core/project-graph');

main();

async function main() {
  const graph = await createProjectGraphAsync()

  console.log(
    JSON.stringify({
      nodes: graph.nodes['@my-org/proj-a'],
      deps: graph.dependencies['@my-org/proj-a'],
    }),
  );
}

• this will result into following node definition:

{
          "file": "packages/proj-a/index.js",
          "hash": "15789608a71660844b9f5822cf3f2ed2c473ddf2",
          "deps": ["@fluentui/proj-b"]
},

• this will result into following dependencies definition:

{ "source": "@my-org/proj-a", "target": "@my-org/proj-b", "type": "static" },

there should be no @my-org/proj-b as a dependency

Failure Logs

Environment

 Node : 14.18.1
   OS   : darwin x64
   yarn : 1.23.34
   
   nx : 13.8.1
   @nrwl/angular : undefined
   @nrwl/cli : 13.8.1
   @nrwl/cypress : undefined
   @nrwl/detox : undefined
   @nrwl/devkit : 13.8.1
   @nrwl/eslint-plugin-nx : undefined
   @nrwl/express : undefined
   @nrwl/jest : 13.8.1
   @nrwl/js : 13.8.1
   @nrwl/linter : 13.8.1
   @nrwl/nest : undefined
   @nrwl/next : undefined
   @nrwl/node : 13.8.1
   @nrwl/nx-cloud : undefined
   @nrwl/react : undefined
   @nrwl/react-native : undefined
   @nrwl/schematics : undefined
   @nrwl/storybook : undefined
   @nrwl/tao : 13.8.1
   @nrwl/web : undefined
   @nrwl/workspace : 13.8.1
   typescript : 4.3.5
   rxjs : 6.6.7

[Hotell]

Updated the issue description, but adding the findings here for better visibility as well:

🎬 I was able to narrow down the AST parsing issue and the culprit is template string! whenever there is one being used and followed by string that includes static import then we run into wrong dependency tree resolution.

// @filename packages/proj-a/index.js

const npmScope = 'myorg';
// 🚨 this is the root cause - template strings interpolation
console.log(`@${npmScope}`);


// 🚨 now your project depends on proj-b ( which is not true )
console.log(`import {foo} from '@my-org/proj-b'`)

following will properly resolve the AST:

// @filename packages/proj-a/index.js

const npmScope = 'myorg';
// ✅ no template strings, AST works
console.log('@' + npmScope);


// ✅ this will do no harm as expected
console.log(`import {foo} from '@my-org/proj-b'`)

[FrozenPandaz]

Yes, I'm pretty sure this is due to the underlying issue of these bugs: microsoft/TypeScript#47597 microsoft/TypeScript#30878

For some reason, the Scanner from typescript, does not identify the right tokens for this syntax. I tried to work around it but wasn't successful.

For your file specifically, I would suggest perhaps using a snapshot? Nx won't analyze those files for dependencies.

[Hotell]

Thanks for additional insights into this issue 🙌!

While I can use file snapshot in this case to mitigate my issue, long term this is quite dangerous ticking bomb for anyone else ( I've spent on it almost 2 days 😅 figuring out what's wrong). Also not fan of file snapshots in general so we try to avoid it. Curious if there isn't a option to use more reliable TS parser?

Thanks!

[Hotell]

UPDATE: I was able to resolve the problem - for whatever strange reason, invoking node script via yarn workspaces @fluentui/nx-workspace-tools check-graph didn't provide the same nx graph result.

When invoking directly from CLI or via nx run-cmd executor, the error is consistent on both CI and Local. any idea why that might be happening @FrozenPandaz ?

---

additional observation - this bug is even more tricky as on CI it parses files differently then on local machine (tested on both Linux local / Linux CI and OSX local / Linux CI )

Data acquired via nx graph --file=output.json

CI

Local:

• on local check passes, dep tree is correct
• on CI it fails https://uifabric.visualstudio.com/fabricpublic/_build/results?buildId=237033&view=logs&j=258ec178-2d8b-5611-7b9b-60c5c95dae55&t=3dbc0aa8-94d2-5f1a-6c00-c4086d74d6b7

[JamesHenry]

Hey @Hotell, hope you are well, mate!

I have implemented a fix for this as well as a few other edge cases in #10360

[github-actions]

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏