webpack-contrib / i18n-webpack-plugin

[DEPRECATED] Embed localization into your bundle
MIT License
318 stars 74 forks source link

Breaking change with 1.0.0 beta, but what is it? #50

Closed StephanBijzitter closed 7 years ago

StephanBijzitter commented 7 years ago
Module parse failed: /vagrant/node_modules/babel-loader/lib/index.js!/vagrant/node_modules/ts-loader/index.js!/vagrant/src/business/Intent/Intent.ts _this.evaluateExpression is not a function
You may need an appropriate loader to handle this file type.
TypeError: _this.evaluateExpression is not a function
    at Parser.<anonymous> (/vagrant/node_modules/i18n-webpack-plugin/dist/index.js:94:31)
    at Parser.applyPluginsBailResult1 (/vagrant/node_modules/tapable/lib/Tapable.js:120:27)
    at Parser.walkCallExpression (/vagrant/node_modules/webpack/lib/Parser.js:1096:19)

I just upgraded from 0.3.0 to 1.0.0-beta.0 and restarted webpack-dev-server to see this error in the console. Note that this was the only change made to trigger this error; no code was changed.

It seems it's crashing on the file because it's a typescript file, but 0.3.0 had no problems here, so I'm wondering if this change is an accidental bug in 1.0.0. If you could point me into the right direction, I might be able solve this issue in a PR, but it'll also be a new experience for me :-)

Using webpack 2.5.1, webpack-dev-server 2.4.5 and NodeJS 6.10.3

hovissimo commented 7 years ago

I got this error with the beta as well, same trace, but I don't use typescript.

Webpack 2.2.1, NodeJS 6.9.1

alexander-akait commented 7 years ago

@StephanBijzitter @hovissimo guys, can your create minimal test reproducible test repo?

joshwiens commented 7 years ago

@StephanBijzitter - 1.0.0-beta doesn't technically change functionality, it does update everything to ES2016+ and enforces engine restrictions in the package.json ( check the changelog in master ) so this is definitely a bug of some sort.

Anything public where that I can get into a debugger would be super helpful to get this addressed.

StephanBijzitter commented 7 years ago

I'll try to set up a repository with minimal code to reproduce on Monday! Edit: didn't seem to find any time, but at least someone made a reproduction case :-)

hovissimo commented 7 years ago

Deleting previous comment as I can now minimally reproduce the error.

@d3viant0ne Please see: https://github.com/hovissimo/w18n

msholty-fd commented 7 years ago

I think the issue is because of _this not being the correct this, because of this: https://github.com/webpack-contrib/i18n-webpack-plugin/blob/master/src/index.js#L48

i18nPlugin is not an arrow function, so when transpiled down, this.evaluateExpression changes to _this.evaluateExpression and the function is not there.

Go into the compiled code (dist/index.js) and change all instances of _this to this and you'll see the issues go away. I don't know exactly how to fix this, because it looks like it was an intentional thing to not use an arrow function here.

EDIT: I had it backwards. I see now that this.evaluateExpression is from the Tapable context, hence you originally didn't use an arrow function.

joshwiens commented 7 years ago

Sorry guys, busy last week. Working on this right now.

joshwiens commented 7 years ago

@hovissimo @StephanBijzitter - Pick up 1.0.0-beta.1, problem is resolved in the project Hovis provided. If anyone else has an issue, ping me on Twitter ( goes to my phone ) so I can get it addressed. I'd like to get a single rc version out and hopefully on the dist-tag by end of week.

screen shot 2017-06-06 at 4 46 41 pm

StephanBijzitter commented 7 years ago

Will try tomorrow! Promise!

Edit: works!

msholty-fd commented 7 years ago

@d3viant0ne I had the issues reported in this thread, and I tested with 1.0.0-beta.1. I don't see any problems now. Thanks a lot!

joshwiens commented 7 years ago

@msholty-fd - No problem, will hold off on resolving it until I hear back from Stephan.

StephanBijzitter commented 7 years ago

Yeah I tried it on our master branch and the warnings for missing translations were gone (which is what we wanted most), but sadly was unable to test it in the typescript pull request so far, anyway as it turned out the problem had nothing to do with typescript, so I'm quite confident everything is a-okay!

joshwiens commented 7 years ago

All - v1.0.0 is now @latest on npm. I'm going to leave the @beta dist-tag in place until next week to given anyone using that tag a chance to move to 1.0.0.

No changes between 1.0.0-beta.1 => 1.0.0. Should just be a matter of changing the package.