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

Fixes --config option processing #509

Merged
merged 2 commits into from
May 22, 2019

Conversation

brandonc
Copy link
Contributor

Overview

Processing process.argv inside getConfig can cause problems when used in conjunction with @lingui/loader since the --config arg is also used by webpack

Instead of processing process.argv, getConfig now accepts the path to a custom config file. Each command now passes the config option from commander into getConfig. Webpack loader now accepts an option "config" that can be used to specify a config file.

Notes

Should all lingui options be allowed via webpack loader options? loader could be modified to assign all options to getConfig output.

I think I fixed an unrelated bug with lingui/loader. compileNamespace and pseudoLocale config options were not being passed to createCompiledCatalog, so I assume that all loader compiled output was being exported as cjs.

Testing

I smoke tested the fix by implementing lingui/loader with a config option while simultaneously using the --config option with webpack. I also verified that lingui extract --config continued to function as expected.

Instead of processing process.argv, getConfig now accepts the path to a custom config file. Processing process.argv can cause problems when used in conjuction with @lingui/loader since the --config arg overlaps with webpack
Copy link
Contributor

@tricoder42 tricoder42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This looks really great 🎉

Should all lingui options be allowed via webpack loader options? loader could be modified to assign all options to getConfig output.

I think having only config is fine. CLI commands doesn't accept all config parameters either.

I think I fixed an unrelated bug with lingui/loader. compileNamespace and pseudoLocale config options were not being passed to createCompiledCatalog, so I assume that all loader compiled output was being exported as cjs.

Ah, good point. I think I was fixing similar issue in next branch, but that was few months ago. I can't remember. Thanks for cleaning it up!

@brandonc
Copy link
Contributor Author

@tricoder42 Thanks! What is the next step?

@tricoder42 tricoder42 merged commit 43873c9 into lingui:master May 22, 2019
@tricoder42
Copy link
Contributor

Sorry I completely forgot. I'll release it in few minutes.

@tricoder42
Copy link
Contributor

Released in v2.8.2.

@semoal
Copy link
Contributor

semoal commented May 22, 2019

Hi guys actually my code is crashing due this release.
Captura de pantalla 2019-05-22 a las 22 46 16

Any suggerence? @tricoder42

@tricoder42
Copy link
Contributor

@semoal Sorry for that. The tests for loader are broken, so this bug sneaked into the release :/

I'll fix it in few minutes.

@tricoder42
Copy link
Contributor

@semoal Fixed in v2.8.3. Thanks for report!

nLight referenced this pull request in dcos/dcos-ui Jun 25, 2019
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 this pull request may close these issues.

3 participants