Skip to content

feat(linter): add support for package based secondary entry points#30809

Merged
meeroslav merged 5 commits intomasterfrom
fix/respect-different-entry-point-for-lazy-check
Apr 25, 2025
Merged

feat(linter): add support for package based secondary entry points#30809
meeroslav merged 5 commits intomasterfrom
fix/respect-different-entry-point-for-lazy-check

Conversation

@meeroslav
Copy link
Copy Markdown
Contributor

@meeroslav meeroslav commented Apr 22, 2025

This PR adds support for package.json based secondary entry points and implements fix for situation when package imports base entry point as dynamic dependency and secondary entry point as static dependency.

Current Behavior

When the package is imported from itself, check for a secondary entry point checks only Angular-style secondary entry points.

When package is importing from the same library as dynamic import from root and static import from secondary entry point we still get linter errror.

Expected Behavior

Check for secondary entry points should also support standard package.json-based entry points.

Importing from the same library as dynamic import from root and static import from secondary entry point should be allowed.

Related Issue(s)

Fixes #18552

@meeroslav meeroslav self-assigned this Apr 22, 2025
@meeroslav meeroslav requested a review from a team as a code owner April 22, 2025 12:31
@meeroslav meeroslav requested a review from leosvelperez April 22, 2025 12:31
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 22, 2025

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

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Apr 25, 2025 10:47am

@nx-cloud
Copy link
Copy Markdown
Contributor

nx-cloud Bot commented Apr 22, 2025

View your CI Pipeline Execution ↗ for commit 0fc58c0.

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-ci ✅ Succeeded 55m 28s View ↗
nx run-many -t check-imports check-commit check... ✅ Succeeded 19s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded 2s View ↗
nx-cloud record -- nx format:check ✅ Succeeded 3s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 1s View ↗
nx documentation ✅ Succeeded 1m 1s View ↗

☁️ Nx Cloud last updated this comment at 2025-04-25 11:35:16 UTC

@meeroslav meeroslav force-pushed the fix/respect-different-entry-point-for-lazy-check branch from 52238cf to 4647209 Compare April 24, 2025 15:18
@meeroslav meeroslav changed the title fix(linter): respect secondary entry points when checking static import feat(linter): add support for package based secondary entry points Apr 24, 2025
@meeroslav meeroslav added type: bug type: enhancement scope: linter Issues related to Eslint support in Nx labels Apr 24, 2025
return [];
}
const entryPaths: Array<{ path: string; file: string }> = [];
for (const [key, value] of Object.entries<string>(exports)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exports property could be a string.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, let me rework that.

if (key !== '.' && key !== './') {
entryPaths.push({
path: joinPathFragments(projectRoot, key),
file: joinPathFragments(projectRoot, value),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value here could be a string, but it could also be an object with conditional exports. It can also be null to disable the entry point from being used.

projectRoot
function getEntryPoint(file: string, projectRoot: string): string {
const packageEntryPoints = getPackageEntryPoints(projectRoot);
const fileEntryPoint = packageEntryPoints.find(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entry points could have patterns (*), which would require a more complex matching to find whether an import matches a given entry point.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's handle this later if users request it. I'm worried this might impact performance significantly so I would not add it unless requested.

for (const [key, value] of Object.entries<string>(exports)) {
if (key !== '.' && key !== './') {
entryPaths.push({
path: joinPathFragments(projectRoot, key),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key could be the condition name of a conditional export.

@meeroslav meeroslav requested a review from leosvelperez April 25, 2025 10:32
@meeroslav meeroslav force-pushed the fix/respect-different-entry-point-for-lazy-check branch from 992695f to 0fc58c0 Compare April 25, 2025 10:34
@meeroslav meeroslav merged commit cd55dfc into master Apr 25, 2025
6 checks passed
@meeroslav meeroslav deleted the fix/respect-different-entry-point-for-lazy-check branch April 25, 2025 12:31
FrozenPandaz pushed a commit that referenced this pull request Apr 25, 2025
…30809)

This PR adds support for package.json based secondary entry points and
implements fix for situation when package imports base entry point as
dynamic dependency and secondary entry point as static dependency.

## Current Behavior
When the package is imported from itself, check for a secondary entry
point checks only Angular-style secondary entry points.

When package is importing from the same library as dynamic import from
root and static import from secondary entry point we still get linter
errror.

## Expected Behavior
Check for secondary entry points should also support standard
package.json-based entry points.

Importing from the same library as dynamic import from root and static
import from secondary entry point should be allowed.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #18552

(cherry picked from commit cd55dfc)
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2025

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.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

scope: linter Issues related to Eslint support in Nx type: bug type: enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ESLint enforce-module-boundaries rule fails with library split in secondary entry points and mixed loading scenarios

2 participants