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

snapshot.managedPaths and snapshot.immutablePaths #6205

Open
vankop opened this issue Jun 9, 2022 · 10 comments · Fixed by #6225
Open

snapshot.managedPaths and snapshot.immutablePaths #6205

vankop opened this issue Jun 9, 2022 · 10 comments · Fixed by #6225

Comments

@vankop
Copy link
Member

vankop commented Jun 9, 2022

Feature to document

https://webpack.js.org/configuration/other-options/#immutablepaths
https://webpack.js.org/configuration/other-options/#managedpaths

Could be a regexp with required capture group, which is path itself, e.g. defaults one https://github.com/webpack/webpack/blob/main/lib/config/defaults.js#L384

Author(s)

Additional information

[ ] I am willing to work on this issue and submit a pull request.

@SystemParadox
Copy link

Please make sure this documentation includes why a capture group is needed because it seems unnecessary and confusing.

@vankop
Copy link
Member Author

vankop commented Jun 9, 2022

as I described it is path itself. e.g. in case

/^(.+?)[\\/]cache[\\/]watchpack-npm-[^\\/]+\.zip[\\/]node_modules[\\/]/

captured path will be (.+?)

@SystemParadox
Copy link

But what does that even mean?

My best guess is that the capture group is meant to be the cache key for the folder but you can also have the regex match only certain files to be included as "managed" within it. But I'm just guessing because this isn't explained anywhere.

@vankop
Copy link
Member Author

vankop commented Jun 9, 2022

they described in docs.. both paths should point to directories. e.g. managesPaths /.+?[\\/]node_modules[\\/]/ should be /(.+?[\\/]node_modules[\\/])/

@SystemParadox
Copy link

Sorry, that still doesn't explain what the capture group is actually for. And that example doesn't have a capture group either!

@chenxsan
Copy link
Member

chenxsan commented Jun 18, 2022

I just checked the source code https://github.com/webpack/webpack/blob/2a58ce7883b42e1ebcfde617ec4a27c7feb035e6/lib/FileSystemInfo.js#L1997, capture group is required to work.

So I think the confusing part is why not just use a normal regexp to match paths in webpack source code?

@SystemParadox
Copy link

SystemParadox commented Jun 18, 2022

Yes indeed, the capture group seems completely unnecessary.

  • If is not needed then it should be removed as it's just confusing
  • If it is needed then the docs need to explain why and give an example of when you would use it - an example that couldn't just be covered with a normal regex match

@chenxsan
Copy link
Member

@SystemParadox I've tried to file a patch so we don't need the capture group.

But unfortunately this seems to be a breaking change. Maybe they can fix it in webpack v6 :)

@SystemParadox
Copy link

SystemParadox commented Jun 23, 2022

Since #15969 has been closed "because it is required", can we please get this documented as to why it is required and what it does.

@chenxsan
Copy link
Member

chenxsan commented Jul 2, 2022

I've merged that pull request but will keep this issue open since it's valid concern in my opinion.

@chenxsan chenxsan reopened this Jul 2, 2022
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 a pull request may close this issue.

3 participants