square/js-jose

Type Error when importing as Commonjs package

Mischala opened this issue · 10 comments

Type error when creating a WebCryptographer object
when importing js-jose as a commonJs package
in both a Webpack and Nodejs projects.

Seems that many features of js-jose depend on being executed in the Global namespace (such as being included in a Script tag) which causes it to break when imported as a package.

This prevents users of the js-jose from practising good dependency management.

import jsjose from 'jose-jwe-jws';
const webCryptographer = jsjose.Jose.WebCryptographer();
TypeError: this.setKeyEncryptionAlgorithm is not a function
    at Object.o [as WebCryptographer] (test/js-jose.spec.js:1:2407)
    at http://0.0.0.0:9883test/js-jose.spec.js:8:22366
    at Context.it (test/js-jose.spec.js:8:22430)

Hi, any opinions on this one? It would be awesome if js-jose was to be able to be used without cluttering the global name space. So making it possible to import it in its own namespace through Webpack, this would really tip the scale for us to use it.

This then would also make the case for us to put in the effort to enable the additional/newer ciphers referenced in #76. Without that, we won't get this package approved with project's standards to be used.

Please, shout out if there's a desire to make it Webpack compatible. We could support in working on that as well.

js-jose was initially designed for use in browsers. I'm happy to do merge a PR which does things differently as long as the examples / unittests work. I'm guessing we would need to ship both, the commonjs sources and pre-packaged files? (disclaimer: I'm no expert in how to package js code, so I'm happy to differ to you).

@alokmenghrajani would you be open to utilising Webpack to make jss-jose more modular, and allow it to run in a webpack (or Node) environment?
If so, I think its possible.

Yes webpack is fine. Just make sure we continue having some kind of pre-packaged thing for tests and people who don't use a package manager.

rgr, wilco. 😄

@alokmenghrajani
I have a branch ready for review, but it appears I don't have rights to push a branch to this repo.

Would you allow me to do that? or would you prefer a PR from a forked repo?

Thanks,
Mischa

PR from a forked repo (the way you did) seems fine. I'll review this tomorrow.

Hiya, Any feedback on #82 ?
keen to make any required changes.

This should be resolved with the merging of #82
After the release, I will run up a few tests in Node, then close this issue