-
Notifications
You must be signed in to change notification settings - Fork 27
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 #5 #17
fixes #5 #17
Conversation
DevelopmentModePlugin.js
Outdated
messages = attributes.messages && util.readMessages(attributes.messages, attributes.developmentLocale); | ||
|
||
this.messagesPath = path.resolve(attributes.messages.replace("[locale]", attributes.developmentLocale)); | ||
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.
Let's remove the trailing space here and double space on = path.resolve
.
Very cool, thanks. |
Also, please make sure to sign-off your commit messages: |
What happens if no |
Signed-off-by: Adam Taylor <adam@appsorcery.com>
if you have no loader specified whatsoever then webpack will fail to create the bundles and print a message like: Module parse failed: if you have some other loader specified for json, then it really depends on the loader behavior. for example for the raw-loader webpack will build the bundles, but when you view a page in your browser that contains a require("globalize") you'll have an error in the console with a message from globalize (i assume) about not being able to parse the provided input and a giant dump of said input. |
Actually there's an issue with my solution above. At some point during my testing i must have placed an actual require("/path/en.json") in my source file which allowed the hot update to take place. without such a require the solution i submitted will only show the changed text on a browser refresh (which is better than a full bundle rebuild, but not ideal or what i intended). in order to do the hot update correctly we need to perform an addDependency (similar to what's being done at line 56) to the file stored in variable this.messagesPath in my changes. But i haven't been able to figure out how to do that in a plugin (in a loader you could just add a new SourceNode with the content). So if someone knows how to perform addDependency correctly i'll happily update this so the functionality is complete. |
Please, give me some moment to review your change (finding free time). Thank you @AppSorcery. |
fixes path in server fallback Signed-off-by: Adam Taylor <adam@appsorcery.com>
Signed-off-by: Adam Taylor <adam@appsorcery.com>
I have reviewed your code and something that occurred to me so far is: The end goal of this change is to allow using the built JS files in node according to your example, but I believe we could make the API a bit more clear. I mean, the change is about managing the compiled i18n data in chunks, not strictly about node vs web. Can we make it so that user actually has control over which chunks the i18n compiled data should go to? I don't have a suggestion now, but user should be empowered to say that i18n compiled data should go in its own separate chunks (default), or in the app chunk, or something else. The interesting thing is that when i18n compiled data is chosen to go in the app chunk, the end result is actually an array of apps (one for each supported locale). What do you think? Does it make sense to you? |
are you familiar with i18n-webpack-plugin? this is actually what i was using before moving to globalize-webpack-plugin. it's only capable of doing static i18n replacements, but if you look at the example you can see that it is actually used the way (i think) you are refering to. you pass in a single language to the plugin and it builds an entire app with that one language. to do multiple languages, you just take advantage of webpack's "parallel build" capability -- each build being a different language (again demonstrated in the example). |
https://github.com/AppSorcery/globalize-webpack-plugin # Conflicts: # PrerenderModePlugin.js # examples/prerendering/server.js
put there to fix an issue where the i18n requires block would be missing the webpack variable if you had a require("globalize") and no uses of globalize within a source file, but it creates a new issue with webpack "No template for dependency: NullDependency" when Globalize is used in a source file... so a different solution is needed...
this modification only works only for the developmentLocale, but could be expanded to work for other files or even changes to the cldr data (but i don't think either make sense...so).
requires your webpack configuration to specify the json-loader for json files: