-
Notifications
You must be signed in to change notification settings - Fork 414
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(webpack): fix usage of webpackResolveOptions conditionally #883
fix(webpack): fix usage of webpackResolveOptions conditionally #883
Conversation
Hi @jpnelson! Could you please fix it in |
@@ -59,7 +59,7 @@ export default function webpack4Loader( | |||
|
|||
// Resolved configuration contains empty list of extensions as a default value | |||
// https://github.com/callstack/linaria/issues/855 | |||
if (webpackResolveOptions.extensions?.length === 0) { | |||
if (webpackResolveOptions && webpackResolveOptions.extensions?.length === 0) { |
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.
Optional chaining should help here: webpackResolveOptions?.extensions?.length === 0)
Good call @Anber , updated now! Thanks |
Hi @jpnelson, thank you for providing us with such a useful library. And I have encountered the same error there. It's splendid to see a PR issued already before my action. But, I would like to share more information with you guys:
Why? Since the So I would like to suggest you lock the version without any range modifier or use the distribution tag like next, beta to avoid confusing conditions like that. Temporarily workaround: package.json {
"resolutions": {
"**/@linaria/webpack-loader": "3.0.0-beta.13",
"**/@linaria/webpack5-loader": "3.0.0-beta.13"
}
} |
Updated: Sorry to ping jpnelson by mistake, it should be @satya164 @Anber. :) |
Can we get a release? this is a pretty big bug since webpack 5 doesn't work at all here |
@ntucker I'll publish the next beta later today or tomorrow. |
When will this fix be released please? |
@Anber could you publish this to latest tag? |
@wmzy done! |
Motivation
There's no existing issue for this as far as I know.
Fixes an issue like this:
Summary
There are some contexts in which the
webpackResolveOptions
might be undefined – eg issues like TypeStrong/ts-loader#1206 .In general the loader handles
this._compilation
not being there, but in practice, breaks because of this line. You can see on line 86 that this is meant to handle the case wherewebpackResolveOptions
is undefined, but it was just missed there (if only we could do it in typescript! But I imagine that's more trouble than it's worth for a webpack loader)You can compare this with
linaria@^2.0.0
to see how it differs in the@linaria/webpack4-loader
Test plan
Tested locally by patching the module with this change