fix(core): use require.resolve instead of randomly matching from our …

[FrozenPandaz]

…project graph

Current Behavior

We resolve any imports which match the name of one of the projects in our project graph.

This is kind of arbitrary because this dependency would only exist in Nx and wouldn't stem from any underlying tool.

Expected Behavior

We use require.resolve to resolve those dependencies. This is more valid because the dependency would exist natively in JS.

Related Issue(s)

Fixes #

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
nx-dev	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Jan 20, 2023 at 10:21PM (UTC)

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit ca8351d. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

✅ Successfully ran 7 targets

• nx run-many -t e2e-macos --parallel=1 --exclude=e2e-detox
• nx affected --target=e2e --base=4c2364864a8a1e0f66d95bb7e338f381075d9ce0 --head=ca8351d97bc9da29d1689592e752b02076a573e5 --exclude=e2e-storybook,e2e-storybook-angular --parallel=1
• nx affected --target=test --base=4c2364864a8a1e0f66d95bb7e338f381075d9ce0 --head=ca8351d97bc9da29d1689592e752b02076a573e5 --parallel=1
• nx affected --target=build --base=4c2364864a8a1e0f66d95bb7e338f381075d9ce0 --head=ca8351d97bc9da29d1689592e752b02076a573e5 --parallel=3
• nx run-many -t root-lint check-imports check-commit check-lock-files depcheck documentation --parallel=1 --no-dte
• nx affected --target=lint --base=4c2364864a8a1e0f66d95bb7e338f381075d9ce0 --head=ca8351d97bc9da29d1689592e752b02076a573e5 --parallel=3
• nx-cloud record -- nx format:check --base=4c2364864a8a1e0f66d95bb7e338f381075d9ce0 --head=ca8351d97bc9da29d1689592e752b02076a573e5

---

Sent with 💌 from NxCloud.

[meeroslav]

Require.resolve is notoriously slow. Can we have some benchmarks to check the impact? Running our linter rule on any medium size (50-100 projects) monorepo should tell us.

In any case I would move it after npm package check as that's cheaper operation.

[AgentEnder]

Do you think its worth keeping the cache? Or I guess require.resolve may have some caching, I know regular require does.

[FrozenPandaz]

require has its own cache:
https://nodejs.org/api/modules.html#modules_caching

But it should also be rare for it to get down there.

[FrozenPandaz]

In any case I would move it after npm package check as that's cheaper operation.

It is already below the npm package check. It's below all of our existing checks actually.

The last line isn't part of the actual check. it sets undefined into the cache is to remember that we couldn't resolve the import for later runs.

[FrozenPandaz]

I did a benchmark and it took ~4ms for 500 unique imports on my machine.

Rule                               | Time (ms) | Relative
:----------------------------------|----------:|--------:
@nrwl/nx/enforce-module-boundaries |     4.352 |   100.0%
Rule | Time (ms) | Relative
:----|----------:|--------:

[meeroslav]

Thank you for the benchmark and code reshuffle. Looks good to me!

[github-actions]

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.