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: support magical comment import(/* webpackChunkName: "d" */ "./d") #2686

Merged
merged 2 commits into from
Apr 10, 2023

Conversation

hyf0
Copy link
Collaborator

@hyf0 hyf0 commented Apr 10, 2023

Summary

  • If you want review the regex, click this link

Related issue (if exists)

Closes #2385

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 Apr 10, 2023

⚠️ No Changeset found

Latest commit: f425571

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@github-actions github-actions bot added the team The issue/pr is created by the member of Rspack. label Apr 10, 2023
@hyf0 hyf0 added this to the Next patch release - 0.1.X milestone Apr 10, 2023
@hyf0 hyf0 requested a review from 9aoy April 10, 2023 13:19
@hyf0 hyf0 added this pull request to the merge queue Apr 10, 2023
Merged via the queue into main with commit 99bec3f Apr 10, 2023
5 checks passed
@hyf0 hyf0 deleted the hyf_webpack_chunk_name branch April 10, 2023 13:28
@hyf0 hyf0 changed the title feat: support magical comment "webpackChunkName" feat: support magical comment import(/* webpackChunkName: "d" */ "./d") Apr 11, 2023
@hyf0 hyf0 changed the title feat: support magical comment import(/* webpackChunkName: "d" */ "./d") feat: support magical comment import(/* webpackChunkName: "d" */ "./d") Apr 11, 2023
@lalexdotcom
Copy link

I would have use a more complex Regex

\/\* (?:web|rs)packChunkName:\s+(["'`]?)((?:\.\/)?(?:\w[\w0-9\/\-]+)(?:[\w0-9\-]))\1\s+\*\/

(2nd group has to be used, 1st one is for quotes)
This one is not perfect either, but it allows:

  • use rspackChunkName or webpackChunkName
  • sub folders (cannot got up as only one dot is allowed at start).
  • ensure that last character is not a slash
  • ensure that opening and closing quotes are the same (or that there's no quotes)
  • allow - and _ in folder/file name

@hyf0
Copy link
Collaborator Author

hyf0 commented Apr 11, 2023

I would have use a more complex Regex

\/\* (?:web|rs)packChunkName:\s+(["'`]?)((?:\.\/)?(?:\w[\w0-9\/\-]+)(?:[\w0-9\-]))\1\s+\*\/

(2nd group has to be used, 1st one is for quotes) This one is not perfect either, but it allows:

  • use rspackChunkName or webpackChunkName
  • sub folders (cannot got up as only one dot is allowed at start).
  • ensure that last character is not a slash
  • ensure that opening and closing quotes are the same (or that there's no quotes)
  • allow - and _ in folder/file name

That looks wonderful. Would you like to open a PR for it? I have identified the code that needs to change in #2688.

@lalexdotcom
Copy link

lalexdotcom commented Apr 11, 2023

I actually don't know Rust, I'm a TS developer...

By looking at the code, I think I could guess where to edit the regex and match group to select, but I tried to fork the repository, and open the devcontainer, but the dev environment doesn't seem to work (pnpm init doesn't work (pnpm/pnpm#5803), and tsc has an issue too...

I could commit the code this way, but I'm not able to test it...

Would you mind to open a PR?

@hyf0
Copy link
Collaborator Author

hyf0 commented Apr 11, 2023

Would you mind to open a PR?

I don't mind at all. Does it block your use cases on Rspack? If it does, I'll happy to open a PR to fix it now. If it doesn't, I would keep #2688 open, which is created as a good first issue on purpose.

@lalexdotcom
Copy link

It's not blocking for my use case, it's just the first thing I tested when I saw your PR merged in 0.1.7.... ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team The issue/pr is created by the member of Rspack.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: support webpackChunkName
3 participants