This repository has been archived by the owner on Oct 1, 2024. It is now read-only.
🐛 Make the webpack plugins return regular objects intead of Modules #618
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amrocha
force-pushed
the
react-i18n-simple-object-import
branch
from
d91ca56
to
43fd2e8
Compare
|
I considered doing a one liner like: return {
...(await import(/* webpackChunkName: "Locales-i18n-[request]" */ `locales/${locale}.json`)),
};But the two-step approach seemed clearer to me. Opinions? |
lemonmade
suggested changes
Mar 29, 2019
|
Oh you're right, I didn't realize we could do that as well. Updated the PR :D |
lemonmade
approved these changes
Mar 29, 2019
| @@ -37,7 +37,8 @@ describe('babel-pluin-react-i18n', () => { | |||
| id: 'MyComponent_${defaultHash}', | |||
| fallback: _en, | |||
| async translations(locale) { | |||
| return await import(/* webpackChunkName: "MyComponent_${defaultHash}-i18n", webpackMode: "lazy-once" */ \`./translations/$\{locale}.json\`); | |||
| const dictionary = await import(/* webpackChunkName: "MyComponent_${defaultHash}-i18n", webpackMode: "lazy-once" */ \`./translations/$\{locale}.json\`); | |||
| return dictionary.default; | |||
There was a problem hiding this comment.
I think it may be safer to do dictionary && dictionary.default since there is not a 100% guarantee a module will be resolved totally correctly (and undefined is a valid value to return)
There was a problem hiding this comment.
That makes sense, especially since this is added to the code post-compile. Done!
amrocha
force-pushed
the
react-i18n-simple-object-import
branch
from
cc8d2ae
to
3bce59e
Compare
amrocha
force-pushed
the
react-i18n-simple-object-import
branch
from
33a2f1c
to
fe3ca32
Compare
amrocha
changed the title
amrocha
changed the title
cartogram
pushed a commit
that referenced
this pull request
Apr 1, 2019
…o hooks__react-shortcuts * 'hooks__react-shortcuts' of github.com:Shopify/quilt: yarn lock feedback Adds useShortcut hook Publish React testing improvements (#621) Publish 🐛Make the webpack plugins return regular objects intead of Modules (#618) Publish @shopify/react-testing (#603) Publish update changelog ✅ updates tests to use GraphQL success response 🐛 check success via valid webhookSubscription field Publish adding changelog entry and docs for #601 Add client submit errors to errors in render prop Publish Update trigger to support act() (#612) Add a promise mock (#614)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the webpack plugin in
react-i18ngives consumers aModuleobject to work with. This isn't what's expected, and this PR changes it to return a simple Object instead.Dynamic
importstatements return object-like data primitives calledModules. These modules behave roughly like an object in terms of access, but don't have any of the standardObjectfunctions since they're not created with anObjectas a prototype.I believe the expectation here is that react-i18n return a standard object for people to manipulate, not a Module, hence this change. For example, this caused a regression in
webwhen@shopify/polariswas updated to use a custom simplifiedmergefunction, instead oflodash's merge function.lodashcould deal withModules, but the new simplified function expects simpleObjects.