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

Work around broken query parsing in merged configs #16

Closed
wants to merge 1 commit into from

Conversation

exogen
Copy link

@exogen exogen commented Sep 20, 2016

This works around a bug somewhere in webpack, another loader in the chain, or our config merging somehow.

In the webpack@1 version of extract-text-webpack-plugin, the loader chain must be a string, like so: css?foo!stylus?bar. It can't be a list of objects like so:

[{ loader: 'css', query: { ... } }, loader: 'stylus', query: { ... } }]

That means query options need to be serialized. For complex nested options, you're supposed to be able to do this, as demonstrated by @sokra:

"stylus?{ foo: 1, bar: 'two' }"
// or more conveniently:
"stylus?" + JSON.stringify(options)

This works in some versions of our archetype's webpack config. In others, we get this error:

ERROR in ./~/css-loader?-autoprefixer!./~/stylus-relative-loader?{"precacheImportVariables":[object Object]}!./client/styles/base.styl
Module build failed: SyntaxError: Unexpected 'o' at line 1 column 30 of the JSON5 data. Still to read: "object Object]}"
    at JSON5.parse.error (/Users/brianbeck/Projects/Walmart/product/node_modules/loader-utils/node_modules/json5/lib/json5.js:56:25)

Notice how the array gets turned into [object Object] – some module somewhere else in the chain is taking our perfectly fine JSON query, parsing it, then doing a terrible job re-serializating it, incorrectly, before it even gets to us.

So this PR introduces extra parsing of the precacheImportVariables option, which lets us specify it as a non-JSON query string by doing an extra parseQuery. 😠

/cc @ananavati @jmcriffey

@dstevensio
Copy link

This saddens me but I'm glad you worked around it.

LGTM 👍

@exogen exogen changed the title Work around broken query parsing somewhere in webpack or a loader Work around broken query parsing in our config merging Sep 20, 2016
@exogen exogen changed the title Work around broken query parsing in our config merging Work around broken query parsing in merged configs Sep 20, 2016
@exogen
Copy link
Author

exogen commented Sep 20, 2016

We can eliminate this workaround by helping with this issue: Fitbit/webpack-config#45

@exogen
Copy link
Author

exogen commented Sep 20, 2016

Fixed in the latest webpack-config 6.1.3!

@exogen exogen closed this Sep 20, 2016
@exogen exogen deleted the fix-webpack-query-parsing branch September 20, 2016 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants