feat(linter): support path wildcards in enforce-module-boundaries autofix

[dearlordylord]

…ofix

What it solves: supports wildcards from https://www.typescriptlang.org/docs/handbook/module-resolution.html#path-mapping in enforce-module-boundaries --fix

Original discussion and bug report https://nrwlcommunity.slack.com/archives/CMFKWPU6Q/p1690292877751699

Current Behavior

only basic case seems to be handled, the one that is generated by default generators

Expected Behavior

More advanced aliasing https://www.typescriptlang.org/docs/handbook/module-resolution.html#path-mapping, including paths: {"@currents/graph-internals/*": ["libs/graph-internals/src/lib/*"],} , is desirable

Related Issue(s)

None

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
nx-dev	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 28, 2023 7:00am

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 7fe6f6d. 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 8 targets

• linux nx affected --target=e2e --base=4c2f98ebf94382bd76baf08e7ad90e6d7f9cc49f --head=7fe6f6d3a2c127c0cbf4fb82a8a9812f452aba9a --parallel=1
• linux nx affected --target=test --base=4c2f98ebf94382bd76baf08e7ad90e6d7f9cc49f --head=7fe6f6d3a2c127c0cbf4fb82a8a9812f452aba9a --parallel=1
• linux nx affected --target=build --base=4c2f98ebf94382bd76baf08e7ad90e6d7f9cc49f --head=7fe6f6d3a2c127c0cbf4fb82a8a9812f452aba9a --parallel=3
• linux nx run-many -t check-imports check-commit check-lock-files check-codeowners documentation --parallel=1 --no-dte
• linux nx affected --target=lint --base=4c2f98ebf94382bd76baf08e7ad90e6d7f9cc49f --head=7fe6f6d3a2c127c0cbf4fb82a8a9812f452aba9a --parallel=3
• nx-cloud record -- nx format:check --base=4c2f98ebf94382bd76baf08e7ad90e6d7f9cc49f --head=7fe6f6d3a2c127c0cbf4fb82a8a9812f452aba9a
• linux nx documentation --no-dte
• macos nx affected -t e2e-macos --parallel=1 --base=4c2f98ebf94382bd76baf08e7ad90e6d7f9cc49f --head=7fe6f6d3a2c127c0cbf4fb82a8a9812f452aba9a

---

Sent with 💌 from NxCloud.

[meeroslav]

Why was this needed? Wasn't relative(...) above already returning a relative path?

[dearlordylord]

it's relative but when there's only one level of relativity (./), relative() would cut ./ making i.e. expected ./my/module into my/module: it goes into imports like this then import { some } from 'my/module' which makes resolver think it's an alias or a node lib, whereas what we really need is import { some } from './my/module' to reflect that we import relatively to the same dir.

[meeroslav]

Wow, that's unexpected behavior of that function. Thanks for the clarification

[meeroslav]

Can we name this something like ensureRelativeForm or something along those lines. This name doesn't really explain what the function does.

[dearlordylord]

ensureRelativeForm sounds good, I didn't come up with a better name. Renamed!

[meeroslav]

I prefer to have these explicitly used. It makes code shorter and more readable as it doesn't encapsulate the extension.

[dearlordylord]

Got it. I'll change it back soon. Although I think that encapsulating extension doesn't harm readability, I respect the project code guidelines.

[dearlordylord]

Inlined

[meeroslav]

Again, please use * directly. It makes the code more readable.

[dearlordylord]

Inlined

[meeroslav]

Can we make this simpler?

The getOneOrNone seems to be doing a bit more than that since it also returns the result of the function. Additionally, if we are using generics, then they should extend from something other than any.

We want to collect all the paths where the alias is a wildcard and matches the import, and then append the delta to the mapped path.

Something like this would work:

const result = new Set<string>; // set ensures there are no duplicates

Object.entries(paths).forEach((alias, targets) => {
  if (!alias.endsWith('*')) {
    return;
  }
  const strippedAlias = alias.slice(0, -1); // remove asterisk
  if (!importScope.startsWith(strippedAlias)) {
    return;
  }
  const dynamicPart = importScope.slice(strippedAlias.length);
  targets.forEach(target => {
    result.add(target.replace('*', dynamicPart)); // add interpolated value
  })
})

return Array.from(result);

I think this is cleaner and more readable, but most importantly - using primitive functions over higher-order functions (map, filter, flat) improves performance significantly.

[dearlordylord]

Used your implementation and validated it against the older code. Looks correct. I took liberty to convert .forEach( into for ... of ... to be in line with removing higher-order functions from the computation.

[meeroslav]

The .forEach is not a higher-order function and is slightly faster than for...of and comparable with plain for. But this is not a deal breaker so let's just leave it.

[meeroslav]

Please replace this recursive logic with:

if (lstatSync(filePath).isDirectory()) {
  // existing unchanged code
} else if (!lstatSync(filePath, { throwIfNoEntry: false })){  // not folder, but not full file either
  // try to find an extension that exists
	const ext = ['.ts', '.tsx', '.js', '.jsx'].find(ext => lstatSync(filePath + ext, { throwIfNoEntry: false }));
  if (ext) {
    filePath += ext;
  }
}

// the rest of the logic unchanged

[dearlordylord]

Replaced

[meeroslav]

The reasoning is correct, but code is unnecessarily complex. Check my comments and try to make it simpler.

[dearlordylord]

Thank you for your suggestions @meeroslav , appreciated it! I've updated the PR accordingly.

[meeroslav]

Looks good! Thank you for the PR and for making the necessary changes.

[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.