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

feat(rspack): NormalModuleReplacementPlugin #6028

Merged

Conversation

xc2
Copy link
Collaborator

@xc2 xc2 commented Mar 26, 2024

Summary

fixes: #3522

thanks #6023

Require Documentation?

  • No
  • Yes, the corresponding rspack-website PR is __

@github-actions github-actions bot added the release: feature release: feature related release(mr only) label Mar 26, 2024
Copy link

netlify bot commented Mar 26, 2024

Deploy Preview for rspack canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 495fac9
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/6602d930d9190b00085e31df

@ahabhgk
Copy link
Collaborator

ahabhgk commented Mar 27, 2024

What about implement it in rust side and expose it to js side by builtin plugin, which has better performance

@ahabhgk
Copy link
Collaborator

ahabhgk commented Mar 28, 2024

I have an idea about this plugin, when the second argument is a string, builtin plugin will have better performance, when it is a function, js plugin (queried hook) will reduce the amount of rust <-> js call, so js plugin will have better performance. Since this is compatible with the PR's implementation, so let's merge this and we can implement the optimization in the future

Copy link
Collaborator

@ahabhgk ahabhgk left a comment

Choose a reason for hiding this comment

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

Thanks

@ahabhgk ahabhgk merged commit 0c3f573 into web-infra-dev:main Mar 28, 2024
27 checks passed
@xc2 xc2 deleted the feat/normal-module-replacement-plugin branch March 28, 2024 13:51
@xc2
Copy link
Collaborator Author

xc2 commented Mar 28, 2024

What about implement it in rust side and expose it to js side by builtin plugin, which has better performance

sry i just seen this.

when it is a function, js plugin (queried hook) will reduce the amount of rust <-> js call,

yes I was wondering if it's worth creating a Rust implementation of this plugin, considering that it seems to be just a wrapper for nmf's xResolve hooks.

a replacement function is more common, and the webpack document of this plugin is also guiding one to modify resource.request and createData.resource instead of returning a new value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: feature release: feature related release(mr only)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NormalModuleReplacementPlugin is not a constructor
2 participants