Skip to content
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

refactor: cleanup CommonJsScanner #5439

Closed
wants to merge 3 commits into from

Conversation

bvanjoi
Copy link
Collaborator

@bvanjoi bvanjoi commented Jan 24, 2024

Fixes #5197

This PR incorporates:

  1. Cleanup of the CommonJsScanner;
  2. Ensuring that RuntimeGlobals::Module is appended in CommonJsSelfReferenceDependency, aligned with the existing Webpack behavior: https://github.com/webpack/webpack/blob/main/lib/dependencies/CommonJsSelfReferenceDependency.js#L129-L131
  3. move get_referenced_exports and get_condition from ModuleDependency to Dependency
  4. remove the ModuleDpenendecy implement for CommonJsSelfReferenceDependency
  5. Add NullDependency trait

ahabhgk
ahabhgk previously approved these changes Jan 24, 2024
@bvanjoi bvanjoi enabled auto-merge (squash) January 24, 2024 05:09
@LingyuCoder
Copy link
Collaborator

Does the issue #5197 resolved by this pr?

@bvanjoi bvanjoi force-pushed the reduce-alloc branch 3 times, most recently from 78c58dd to 1a890a0 Compare January 25, 2024 08:04
@bvanjoi bvanjoi marked this pull request as draft January 26, 2024 04:01
auto-merge was automatically disabled January 26, 2024 04:01

Pull request was converted to draft

Copy link

stale bot commented Mar 26, 2024

This pull request has been automatically marked as stale because it has not had recent activity. If this pull request is still relevant, please leave any comment (for example, "bump").

@stale stale bot added the stale label Mar 26, 2024
@hardfist
Copy link
Contributor

close for outdated

@hardfist hardfist closed this May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should not parse require() in mjs+esm
4 participants