-
Notifications
You must be signed in to change notification settings - Fork 17
ESM/CSJ build & dependency reduction #42
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
Conversation
488e053 to
724ff55
Compare
|
Hi @jeffreyparker @AaronAtDuo ! any chance to review this and have your thoughts on this? Happy to work through any details and make necessary adjustments. |
|
Taking a look |
|
Thanks @AaronAtDuo I have replaced couple of additional node deps with |
This seems fine to me with the EOL coming up, especially given a question I'll get into momentarily. I'm very far from being a JS/TS expert so please bear with me if these are dumb questions:
|
113820e to
daedfc8
Compare
|
thank you @AaronAtDuo for having a look at this!
We're swapping only package manager from Since you mention this, there is new open source registry getting quite a lot a traction in community - JSR (https://jsr.io), you might consider publishing the package to this one too? I think it would just few lines in github action and for you to set up account there in case you'd like to do that.
Yes, it is indeed a breaking change along with dropping Node.js 18 in a semver sense. Swap to async for that method was not much about async being trendy, it's just that I have update the PR description to include node.js 18 drop also updates dependencies to reflect this. |
|
@AaronAtDuo @jeffreyparker Hi both, I was wondering if you had a chance to give this a look? Happy to work through any further required changes. |
|
@lukashroch Finally getting back to this, sorry for the delay. First thing, just looks like some merge conflicts popped up. Also, we noticed that this diff seems to have undone a package file fix in https://github.com/duosecurity/duo_universal_nodejs/pull/30/files? |
- support esm/cjs treeshakable without transient dependencies to support various envs
- tsup and extracts version as constant from the json file
- example as pnpm-managed workspace - update dependencies - drop EOL node.js 18
|
@AaronAtDuo thank you for reviewing! I have now resolved the conflicts that accumulated over time and bumped all deps as it was stale for a bit. Re package file fix: Yes, it was intentional to 1) get rid of the usage of node.js libs to be more env agnostic, 2) also it should not be needed, build system extracts it const now, so there shouldn't be a need to import package.json file like this. if you have a look into the build artefacts, you'll see it like this: |
|
@lukashroch I'll try to get a (major version) release out within the next couple of days. Thanks so much for your contributions - and patience! |
Description
tscbuild process changed to usetsup- ESM/CJS supportjsonwebtokentojose- supports ESM/CJS, treeshakable, wide range of envs and no dependencies (solved Using in Vite/Vue3 App Throws 'Uncaught TypeError: util.inherits is not a function' #15,jsonwebtokenusesjwsand other deps which rely on couple of node libs)ENOENT: no such file or directoryError when trying to createClient#36pnpm(faster and better deps tree resolution, npm already had some issues... ) and used lib build for example directly, github actions adopted topnpmtoovitestMotivation and Context
tsconly (Using in Vite/Vue3 App Throws 'Uncaught TypeError: util.inherits is not a function' #15, NextJSENOENT: no such file or directoryError when trying to createClient#36 )How Has This Been Tested?
Types of Changes
Function
client.createAuthUrl('username', 'state')now returns a promise.Drop Node.js 18 -> EOL