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

fix: require.cache undefined #2468

Merged
merged 14 commits into from
Mar 31, 2023

Conversation

faga295
Copy link
Contributor

@faga295 faga295 commented Mar 26, 2023

Summary

closed #2460

Currently, require.cache will transform to __webpack_require__.c, but I'm confusing about where to insert runtime_globals::MODULE_CACHE to runtime_requirement.

Since &mut runtime_requirement is borrowed to inject_runtime_helper, I can't find a good solution unless move transform require.cache operation to inject_runtime_helper and insert MODULE_CACHE in it.

Perhaps my confusion comes from my lack of familiarity with rust, and if so, please forgive me, I'm trying to learn.

Related issue (if exists)

Types of changes

  • Docs change / Dependency upgrade
  • Bug fix
  • New feature / Improvement
  • Refactoring
  • Breaking change

Checklist

  • I have added changeset via pnpm run changeset.
  • I have added tests to cover my changes.

@changeset-bot
Copy link

changeset-bot bot commented Mar 26, 2023

🦋 Changeset detected

Latest commit: 63adda6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 18 packages
Name Type
@rspack/binding Patch
@rspack/postcss-loader Patch
@rspack/core Patch
webpack-test Patch
@rspack/playground Patch
@rspack/cli Patch
@rspack/dev-middleware Patch
@rspack/dev-server Patch
@rspack/plugin-html Patch
benchmarkcase-rspack-react-refresh Patch
@rspack/dev-client Patch
@rspack/plugin-minify Patch
@rspack/plugin-node-polyfill Patch
@rspack/binding-darwin-arm64 Patch
@rspack/binding-darwin-x64 Patch
@rspack/binding-linux-x64-gnu Patch
@rspack/binding-win32-x64-msvc Patch
@rspack/fs Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@CLAassistant
Copy link

CLAassistant commented Mar 26, 2023

CLA assistant check
All committers have signed the CLA.

@faga295 faga295 marked this pull request as ready for review March 26, 2023 11:32
@underfin
Copy link
Collaborator

@faga295 Could you refactor the code using const dependency? here is an example https://github.com/web-infra-dev/rspack/blob/main/crates/rspack_plugin_javascript/src/visitors/dependency/scanner.rs#L336

@faga295
Copy link
Contributor Author

faga295 commented Mar 30, 2023

@faga295 Could you refactor the code using const dependency? here is an example https://github.com/web-infra-dev/rspack/blob/main/crates/rspack_plugin_javascript/src/visitors/dependency/scanner.rs#L336

Ok, I have refactored it, please review it.

underfin
underfin previously approved these changes Mar 31, 2023
@underfin
Copy link
Collaborator

underfin commented Mar 31, 2023

faga seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

@faga295 The cla is not signed. Look like your name of commit is not equal to your github name, Could you check it?

@faga295
Copy link
Contributor Author

faga295 commented Mar 31, 2023

thank you for your remind, it works now.

@underfin
Copy link
Collaborator

@faga295 Look like you need to rebase the main branch, because runtime_globals moved to RuntimeGlobals. Look like cargo clippy crashed in ci, you can run cargo clean && cargo clippy in local environment to check it.

@faga295 faga295 requested a review from underfin March 31, 2023 07:29
@underfin
Copy link
Collaborator

@faga295 I fixed clippy error at my commit. It colud run ci success.

@underfin
Copy link
Collaborator

Thanks a lot!

@underfin underfin enabled auto-merge March 31, 2023 08:39
@underfin underfin added this pull request to the merge queue Mar 31, 2023
Merged via the queue into web-infra-dev:main with commit 60e0aec Mar 31, 2023
5 checks passed
@github-actions github-actions bot mentioned this pull request Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report]: require.cache not available in CJS modules
5 participants