Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1 of 4 tasks
edbzn opened this issue Aug 9, 2023 · 17 comments · Fixed by #30809
Closed
1 of 4 tasks
Assignees
Labels
outdated scope: linter Issues related to Eslint support in Nx type: bug

Comments

@edbzn
Copy link
Contributor

edbzn commented Aug 9, 2023

Current Behavior

The ESLint rule enforce-module-boundaries enforce libs to be lazy loaded as far as a dynamic import is detected. This is great for most scenarios, but not working well when a library is split into multiple secondary entry points.

Consider the following scenario where lib A is split into 2 secondary entry points core and feature, and lib B needs to lazy load feature and to statically load core:

image

Then the enforce-module-boundaries rule is failing because lib A is marked as "lazy", and everything should be lazy loaded as far as a dynamic import is detected.

import { CommonModule } from '@angular/common';
import { NgModule } from '@angular/core';

/* Here is the line that throws "Static imports of lazy-loaded libraries are forbidden."
   but static import of the secondary entry point "core" should be allowed. */
import { CoreModule } from '@myorg/lib-a/core';

/* Only "feature" is lazy, not the whole library and all the secondary entry-points. */
async function getFeatureModule() {
  const  { FeatureModule } = await import('@myorg/lib-a/feature');
  return FeatureModule;
}

getFeatureModule();

@NgModule({
  imports: [CommonModule, CoreModule],
})
export class LibBModule {
}

Expected Behavior

Statically importing core secondary entry points should not throw static imports of lazy-loaded libraries are forbidden.

Secondary entry points should be taken into account when checking for static imports when a library is considered lazy, allowing the rule to statically import secondary entry points that have not been imported dynamically.

GitHub Repo

https://github.com/edbzn/nx-static-import-issue/tree/main

Steps to Reproduce

  1. Clone the reproduction
  2. Run nx lint lib-b
  3. See the line that is failing

Nx Report

nx 16.6.0

Failure Logs

No response

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

No response

@edbzn edbzn added the type: bug label Aug 9, 2023
@AgentEnder AgentEnder added the scope: linter Issues related to Eslint support in Nx label Aug 11, 2023
@meeroslav
Copy link
Contributor

Thank you @edbzn for reporting this.

To be honest, this was on our list for a while. We will get to it as soon as possible.

@plutotv-aaa
Copy link

Thank you for working on this. Is there any update @meeroslav? Much appreciated.

@meeroslav
Copy link
Contributor

Sadly not yet

@montella1507
Copy link

montella1507 commented Dec 19, 2023

Hi i have come here to create issue when i saw i had been already created.

This is really common for shared libraries with secondary entry points as a code-splitting / tree-shaking implementation.

We have several huge applications with libraries (@company/ui for the instance) which are usually small and eagerly loaded (in HTML template of parent components, imported etc)...

however there are huge components too, which are usually loaded lazily (with custom dyanimc-component, dynamic import(), or... with the latest defer() angular feature).

import('@company/ui/secondary-entry-point')

defer() is at the moment not possible to use as far as the component is usually loaded as secondary entry point...

@meeroslav is it possible to escalate this issue, pllease?

@meeroslav meeroslav self-assigned this Dec 20, 2023
@de-don
Copy link

de-don commented Jan 17, 2024

+1

@tiddlypip
Copy link

tiddlypip commented Jan 17, 2024

Looks like theres a checkDynamicDependenciesExceptions option, seems you can add lazy loaded imports to this but haven't had the opportunity to try it yet: https://nx.dev/nx-api/eslint-plugin/documents/enforce-module-boundaries

@montella1507
Copy link

montella1507 commented Jan 18, 2024

Yeah, however this will lead to 40 exceptions in our case as far as we have /ui library with lot of components and secondary entry points and ...as far as defer does not work effectively without that and we have lot of "3rd party" huge libraries involved in them (spreadsheets etc.)

IT does not sound so "exceptional" then :D

@supernaut
Copy link

Not sure if this would cover it, but similarly I would love for the @nx/enforce-module-boundaries rule to be enforced on components in Next.js libraries with 'use client' but not server components.

@wizardnet972
Copy link
Contributor

Any updates?

1 similar comment
@frozenwings
Copy link

Any updates?

@joarkm1
Copy link

joarkm1 commented Aug 30, 2024

I see that secondary entry points do not appear as separate nodes in nx graph. Maybe they should be?

@pvds
Copy link
Contributor

pvds commented Oct 8, 2024

Thank you @edbzn for reporting this.

To be honest, this was on our list for a while. We will get to it as soon as possible.

HI @meeroslav is this issue planned or on any roadmap?
Would be awesome if NX can also "understand" secondary entrypoints!

Copy link

github-actions bot commented Apr 7, 2025

This issue has been automatically marked as stale because it hasn't had any activity for 6 months.
Many things may have changed within this time. The issue may have already been fixed or it may not be relevant anymore.
If at this point, this is still an issue, please respond with updated information.
It will be closed in 21 days if no further activity occurs.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label Apr 7, 2025
@montella1507
Copy link

@joarkm1

I see that secondary entry points do not appear as separate nodes in nx graph. Maybe they should be?

Please no. Or opt-in. Every bigger monorepo has hundreds of SEPs...

@meeroslav
Copy link
Contributor

meeroslav commented Apr 22, 2025

This issue will have to be fixed with two PRs.

The first one (#30809) solves the problem for Angular, which has a clearly defined secondary entry point definition.

Follow-up for standard ESM packages should include general support for secondary entry points.

See update below 👇

@meeroslav
Copy link
Contributor

Decided to fix everything in a single PR:

  • Add support for package.json-based entry points
    • Include parsing of conditionals and nested exports
  • Check if the static import belongs to the secondary entry point before raising the Static imports of lazy-loaded libraries... error

Please note that right now, our graph does not track the exact import expression. We only store the project name and type of import. This limits us to covering all corner cases. For example, importing from the same secondary entry point both statically and dynamically would not be flagged as an error.

FrozenPandaz pushed a commit that referenced this issue 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)
Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: linter Issues related to Eslint support in Nx type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.