-
Notifications
You must be signed in to change notification settings - Fork 843
v1.5.0 not compatible with production builds in webpack #561
Comments
Bummer -- I had it out as beta for 2+ weeks looking for feedback. |
Do you have a public repo that I look at to reproduce this issue? |
Sorry bud. Didn't even notice the beta. |
That'd be great -- if you can provide me a sample showing how you consume this (i.e. a working 1.4.x version), it'd be a help. Most folks I know only grab the ~/dist file and run from there. |
@DanielSchaffer -- perhaps something about the change in how we export broke this? Any insight? |
Related possibly: #562 |
It looks like from the error that their build is pulling in the ES Modules ( @PaulInglis, what build tool/bundler are you using, what ES level are you targeting, and are you using Babel? |
Webpack 3 and transpiling from ES6 We use the direct import and don't target specific files. |
Can you share your webpack config? |
Production webpack.config
package.json:
|
Similar problem here, when updating to version 1.5.0 it broke the build:
|
Same problem:
|
I resolved this error by manually installing latest version of UglifyJS plugin and setting |
@xakep139 mind showing me what you did, or send a PR? |
@brockallen, sure! I did the following:
|
@xakep139 You can also upgrade to webpack 4.0, which solves this issue. edit: webpack 4 already uses the latest version of uglifyjs |
So @PaulInglis you're not even pulling in the version from ~/lib or ~/dist, right? |
@DarkCow you seem to follow what's happening here, so mind helping me understand. It sounds like you think by me upgrading to webpack 4 that has caused others still on webpack 3 to see this issue? Or am I missing something. |
@brockallen nope - I'm not even using UglifyJS (see the commented out line in the webpack.config above). It seems to be some internal thing if using the My work around is to remove the |
So then I'm still confused... I use webpack (from gulp) to build two versions of this library -- one in ~/lib and one in ~/dist, so if you're not using those then my use of webpack is unrelated to your error in your build using webpack -- I think. right? So I guess what I'm confused about is what happened in how I'm building/packaging this that affected you? That's why I asked @DanielSchaffer to chime in here, as he had a PR to change how exports are done slightly. |
Actually, I take it back -- "main" points to ~/lib/oidc-client.min.js. So if you're doing a require then that's what you're getting. |
Hey, sorry, I'm not ignoring the pings ... had a long weekend with the fam, about to start poking now. The way I understand things, the It seems like it's possible that this is not the case, or at least for Webpack <= 3 when using Uglify, which requires setting the |
Minimal repro and fixes for webpack 3 and 4: https://github.com/DanielSchaffer/oidc-client-webpack |
^^ updated to add fixes and a README |
@brockallen it appears you are doing it correctly? JS devops are so confusing and change a million miles an hour... It appears However, other people using webpack, should be transpiling their code for the browsers they are targeting. IMO you shouldn't be depending too heavily on 3rd party package maintainers to that... In webpack 3, you should be transpiling your code before minification. The nice thing about webpack 4, is that it uses the newest uglifyjs, thus making minification not require transpilation. |
@DanielSchaffer Ahh, that makes sense now about how webpack is resolving the dependency via the module property in the package.json. Ideally updating the webpack config to transpile the code when webpack is creating vendor.js would be the best solution, but in the case of angular cli they don’t directly expose webpack.config.js to the developer. |
So @DanielSchaffer, it was the addition of "module": "index.js", in project.json that is the culprit behind this. When I remove that from your repo, then it's working again. This simply means that importing this (for now) pulls in the transpiled version, and as is evident from this thread it was a breaking change. So for now I'm going to remove it to get people back up and running. I'm happy to start a 2.0 version that changes how this is done, and make that the breaking change. How's that for a solution and a plan to move forward? |
@PaulInglis and @larlew I pushed a "1.5.1-beta.1" with the change I mentioned above. Can you test it, please to see if you're back up and running? |
@brockallen can you change the |
@brockallen - compiles fine for me (didn't before). Not done a thorough test yet (will do it tomorrow, when I have time), but I seemed to be able to hit the site with the cached token. |
@DanielSchaffer wouldn't that also be a breaking change from what was in 1.4? |
@brockallen I don't think so. The issue here is with how the various packaging tools resolve 3rd party libraries. I'm not familiar with how Rollup works, but for Webpack, it uses the
|
Users of create-react-app can't change the webpack version (not without ejecting) and thus the proposed solution of changing webpack's config/version is not suitable. Please provide es5 version for build |
@hawkeng Lastest version (1.5.1-beta.1) works fine with create-react-app and you don't need to touch webpack. This is due to the removal of the |
1.5.1 pushed |
The build of our product started failing because it pulled in v1.5.0 of oidc-client.
We've got our package.json set to
"oidc-client": "^1.4.1",
and it seemed to update it. Clearly it isn't a compatible version (that's what the hat symbolises).Not sure what has caused it - maybe the move to an optional dependency, but it looks as though the file isn't transpiled in the distributed code.
Here is the output of our logs:
The text was updated successfully, but these errors were encountered: