-
Notifications
You must be signed in to change notification settings - Fork 69
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
update dependencies #227
base: v5
Are you sure you want to change the base?
update dependencies #227
Conversation
835d3df
to
03b38ad
Compare
Webpack 4 tests are failing. Looks like we are unable to use See discussion. |
One possible solution here is to merge just the 2nd commit which inlines That doesn't fix |
@@ -128,7 +158,7 @@ function valueProcessor({ join, root, directory }) { | |||
* @return {boolean} True for absolute uri | |||
*/ | |||
function testIsAbsolute(uri) { | |||
return !!uri && (typeof root === 'string') && loaderUtils.isUrlRequest(uri, root) && | |||
return !!uri && (typeof root === 'string') && isUrlRequest(uri, root) && |
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.
@bholloway why cannot we use the loaderUtils.isUrlRequest by just upgrading the loader utils version?
Hello @bholloway, they apparently backported a fix to all their major version, can we just get the hotfix that doesn't suppress options ? Choose your weapon: https://github.com/webpack/loader-utils/releases Proposed update : and nothing else is needed. |
I've proposed a fix for version 3 in #229. |
looking again at this now |
03b38ad
to
6c543c9
Compare
The Version 5.0.0 allows Version 4.0.0 allows I don't think there's any urgency to release changes to these versions. |
Hmm yes good point @orien |
Current state of this change is that
I will put this on hold and come back to it later but (2) might block any later work to update dependencies |
Fixes #226