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: add buildModule hook #3466

Merged
merged 3 commits into from
Jun 13, 2023

Conversation

dmitry-stepanenko
Copy link
Contributor

@dmitry-stepanenko dmitry-stepanenko commented Jun 7, 2023

Related issue (if exists)

#3158

Summary

🤖 Generated by Copilot at a006f49

This pull request adds a new hook buildModule to the compiler that allows plugins to customize the building of JavaScript modules. It implements the hook in both the Rust and the JavaScript layer of the compiler and adds a test case to verify its functionality. It also refactors some code related to the JsModule struct and the Hook enum.

Walkthrough

🤖 Generated by Copilot at a006f49

  • Add a new buildModule hook to the Hook enum and the From<&str> implementation for Hook in crates/node_binding/src/hook.rs (link,link)
  • Add a new build_module field to the JsHooks struct and a new build_module_tsfn field to the Plugin struct in crates/node_binding/src/plugins/mod.rs (link,link)
  • Add a new build_module method to the Plugin struct and the PluginDriver trait implementation for Plugin in crates/node_binding/src/plugins/mod.rs (link,link)
  • Initialize and release the build_module_tsfn field in the Plugin constructor and destructor in crates/node_binding/src/plugins/mod.rs (link,link)
  • Move the JsModule struct definition from crates/node_binding/src/js_values/module.rs to crates/node_binding/src/hook.rs and remove the unused original_source field (link,link)
  • Add a new identifier method to the JsModule struct in crates/node_binding/src/hook.rs (link)
  • Enable tracing for the build_module method in the PluginDriver trait implementation for PluginDriverImpl in crates/rspack_core/src/plugin/plugin_driver.rs (link)
  • Add a new buildModule field to the Compilation class and initialize it with a new SyncHook instance in packages/rspack/src/compilation.ts (link,link)
  • Remove the redundant identifier method from the JsModule class in packages/rspack/src/compilation.ts (link)
  • Add the buildModule method to the hooks object and the hookMap object in the Compiler class in packages/rspack/src/compiler.ts (link,link)
  • Add a new buildModule method to the Compiler class in packages/rspack/src/compiler.ts that calls the buildModule hook and updates the disabled hooks state (link)
  • Add a new test case for the buildModule hook in packages/rspack/tests/configCases/hooks/build-module directory, consisting of a.js, index.js, and webpack.config.js files (link,link,link)

@dmitry-stepanenko dmitry-stepanenko marked this pull request as draft June 7, 2023 15:18
@h-a-n-a h-a-n-a self-assigned this Jun 8, 2023
@h-a-n-a
Copy link
Contributor

h-a-n-a commented Jun 8, 2023

You can ping me as soon as you finish this PR or have any questions.

@dmitry-stepanenko
Copy link
Contributor Author

You can ping me as soon as you finish this PR or have any questions.

@h-a-n-a thank you, actually I'm facing a problem there. One of the things that I need there is identifier() function to be defined on JsModule. To do that, I’m converting it from an interface to a class here https://github.com/web-infra-dev/rspack/pull/3466/files#diff-95c957e3bc52dfdce0db5276ec60213a6223a2dacce1659810127be853604b47 by updating napi attribute from #[napi(object)] to #[napi]. However this leads to an issue when getModules() method here packages/rspack/src/compilation.ts returns empty objects instead of actual modules with their props.

Can you please point me what I’m missing there? The only thing I need is just an idenitifer method that returns self.module_indentifier.to_string().

@dmitry-stepanenko dmitry-stepanenko marked this pull request as ready for review June 8, 2023 13:00
@dmitry-stepanenko
Copy link
Contributor Author

@h-a-n-a it is ready for review

h-a-n-a
h-a-n-a previously approved these changes Jun 13, 2023
@h-a-n-a h-a-n-a enabled auto-merge June 13, 2023 11:26
@h-a-n-a h-a-n-a disabled auto-merge June 13, 2023 11:34
@h-a-n-a h-a-n-a enabled auto-merge June 13, 2023 11:38
@h-a-n-a h-a-n-a added this pull request to the merge queue Jun 13, 2023
Merged via the queue into web-infra-dev:main with commit b940b48 Jun 13, 2023
@hardfist hardfist added need documentation Create a tracking issue in rspack-website and removed need documentation Create a tracking issue in rspack-website labels Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need documentation Create a tracking issue in rspack-website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants