[Beta6]: package.json wrongly points to index.js about oidc-client-js HOT 18 CLOSED

The file in ~/dist is already minified and intended to be used when you want to just include it as a loose file. I think the main in index.js is for folks that want to use this as an ES6 module.

from oidc-client-js.

I understand the intention. It's just that builders like webpack load npm modules referencing the main entry point of package.json. It expects the files to already be transpiled and minified, hence the
exclude: /node_modules/ in webpack.config.js. In general, every npm module I use follows this convention. This would be the first not to do so. I would have to create a build configuration for every external module I want to use (and make a guess what kind of JavaScript they are using), if this convention wasn't followed. This is hardly viable. Please reconsider applying this PM.

from oidc-client-js.

I'm more than happy to consider it (thus the conversation). I'm just trying to learn how people use the packages.

I'm still confused then how someone who was just using ES6 or TypeScript would then load this -- do they not rely upon the module exported by index.js? By making this change are we excluding some other scenario?

from oidc-client-js.

Don't worry - you're most likely not the only one who struggles with npm at some point. I've had my share of sleepless nights over this.

The point you make about TypeScript/ES6 is exactly why package.json should point to the transpiled/minified version (vanilla JavaScript). Then it doesn't matter what configuration the consumer uses - it's all being broken down to the lowest common denominator and vanilla JS can be used by everyone.

I've already tried modifying the entry point and it worked - until I tried to include it in my project using require. I've changed the target to use umd instead of var. This promises (as in I don't know if it really works...) to allow both classic imports using HTML files as well as imports using require. The PR has been updated. Unit tests all passing.

from oidc-client-js.

Forgot to mention that I've also removed babel-polyfill for the time being. It was only required as an entry point in webpack.config.js and caused my project to crash since my project also has a dependency on babel-polyfill and only one instance is allowed to be loaded at the same time. If it is really required, I suggest adding it as a peer dependency.

from oidc-client-js.

For the babel-polyfill, I added that so the ~/dist folder would have a single file that could be used directly by folks that don't know or use npm or anything else. IOW, they could just grab the .js file directly from github and copy it into their projects. With it gone, I suspect things like promises won't work so well in their code (for IE, for example).

from oidc-client-js.

I understand but in that case it crashes when someone is using the package via npm. I guess this is another topic for a sleepless night :)

from oidc-client-js.

Yea, maybe I do 2 builds then -- one that includes the polyfill, one without.

from oidc-client-js.

I can confirm that using my build works at least for using this package with npm & webpack. The project with the current settings compiles without quarrels. However I've realized that the package size went up drastically in comparison to the oidc-token-manager. It stands at a whopping 326kb - in comparison to the 98kb of the token manager.

from oidc-client-js.

FYI I've pushed another update to the PR containing a build script for the classic build process used prior to my changes - npm run build:classic. This produces the file anyone not using npm can just grab from github and include in their project. I've also reincluded babel-polyfill - this time as a peer dependency. This means every user of this module has to provide their own version of the polyfill, compliant with the version in package.json. This should prevent this dependency to accidentally overwrite an already existing module in another project and thus crash the whole build.

I've also added a prebuild and pretest script to package.json to automatically trigger the whole build process when needed. Let me know when you are ready to merge the PR or if there is anything else you need before this can be merged.

About the size: I've seen that jsrsassign's npm module is packed into a single file. This means that you always include everything from jsrsassign, even if you just need a few parts. Sadly the only way I see around this is either ask them to split up their npm module, or pull the files from every part you need from their github and include it in a lib folder.

from oidc-client-js.

Aonther update: I've managed to extract the dependencies from jsrsassign into a lib folder as described above. The process is a bit messy since I had to edit the files in order for the global variables to be present and had to add a module.exports statement at the bottom of each file to be able to require them. As far as I've seen, the only source file requiring the library is JoseUtil.js. I've changed the imports there to use the lib folder instad of the npm module.

The good:
Build size is down to 149kb for the npm module, 230kb for the classic file.

The bad:
Unit tests are failing & I don't know why. I've checked the namespaces and the functions to be called when the tests fail are definitely present, there is just something I'm missing.

The ugly:
It is a complete mess I'm afraid. The lib files require their dependencies to be available in the global namespace and a lot of guesswork is required to see which variables are present and which ones aren't. Might be a good idea to ask the maintainer of the jsrsassign library to split up his code - this is not something we can reliably do without breaking everything.

What should I do with this? When I push these changes to my fork they will automatically be included in the PR. This is not something I want to do. You could merge the PR and then I could push it to my fork for you to look at if you're interested.

from oidc-client-js.

Sorry for going dark here -- I'm traveling this week, so I won't have a chance to look at the PR until this weekend at the earliest. For now, I'd not try to fix the jsrsasign library -- someone already has an open issue with him to help modernize his packaging, but he (like me) is not that familiar with the tooling ecosystem. He'd need a lot of help. I think he could benefit from your help, though. It'd better to fix his stuff, then try to fix it here I think.

So... maybe let's back out this last step from the PR. I'll try to download it before I get on my flight home and have a look.

from oidc-client-js.

Ok I'll have a look when I have the time. Save travels!

from oidc-client-js.

Just hit this exact issue using npm and webpack when I was trying to switch over from the oidc-token-manager. Looking forward to the PR fix for it getting pushed out to npm.

from oidc-client-js.

Yep -- we'll get the right thing done. I just want to understand it all before I merge it in.

from oidc-client-js.

As you might have seen, I started looking and had some questions. I ask them because there are things I'm not familiar with. Thx.

from oidc-client-js.

Ok, thanks for the help on this. I merged the changes. Again, any feedback welcome on the slight changes I made.

from oidc-client-js.

Closing this as the build succeeded in my project. Thanks again!

from oidc-client-js.