-
-
Notifications
You must be signed in to change notification settings - Fork 490
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
Minimal changes necessary for Webpack 5 compatibility #401
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
questions below
Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Judahmeek)
src/utils/processStyleLoaders.js, line 66 at r1 (raw file):
// eslint-disable-next-line no-param-reassign loadersWithSuffix[sassLoaderIndex] = sassLoaderWithSourceMap;
- Will this break compat with older webpack?
- Can you be sure to put in changelog?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @justin808)
src/utils/processStyleLoaders.js, line 66 at r1 (raw file):
Previously, justin808 (Justin Gordon) wrote…
- Will this break compat with older webpack?
- Can you be sure to put in changelog?
-
No, because sass-loader's latest versions already get their default sourceMap config from the
devtools
configuration. -
Sure. How exactly do you want me to phrase it?
bootstrap-loader no longer triggers sourceMap for sass-loader
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Judahmeek)
src/utils/processStyleLoaders.js, line 66 at r1 (raw file):
Previously, Judahmeek (Judah Meek) wrote…
No, because sass-loader's latest versions already get their default sourceMap config from the
devtools
configuration.Sure. How exactly do you want me to phrase it?
bootstrap-loader no longer triggers sourceMap for sass-loader
?
Sure -- I really have zero idea on this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Judahmeek)
src/utils/processStyleLoaders.js, line 66 at r1 (raw file):
Previously, justin808 (Justin Gordon) wrote…
Sure -- I really have zero idea on this one.
maybe just a changelog entry!
maybe a major version bump also!
Please update the https://github.com/shakacode/bootstrap-loader/blob/master/CHANGELOG.md and I will merge and release. |
Same as #400 without the
lib
directory.This change is![Reviewable](https://camo.githubusercontent.com/23b05f5fb48215c989e92cc44cf6512512d083132bd3daf689867c8d9d386888/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)