-
Notifications
You must be signed in to change notification settings - Fork 127
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
build(governance-sdk): generate commonjs and esmodules builds #491
base: main
Are you sure you want to change the base?
Conversation
@johnrees is attempting to deploy a commit to the Solana Team on Vercel. A member of the Team first needs to authorize it. |
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/solana-labs/oyster-governance/9qGcBzcp3g3sJx9w95EjvWYgfJL3 |
Sounds like a good to have to me, but vercel doesn't like it :) |
I'm actually a bit curious about why vercel is building the Shouldn't this be a job for dedicated CI tooling like github actions or travis? Or does it have some frontend code that I'm not aware of? Thanks 🙏 |
ah, I think I get it, nvm. It's building it alongside the packages/governance frontend |
temporary workaround to make it less annoying than having to include `@babel/plugin-proposal-class-properties` (https://git.io/vb4SL) with webpack builds
d78abee
to
8c109da
Compare
Another suggestion for a potentially nice but definitely not required feature which could make things a little easier for consumers of the governance-sdk npm package
this generates an esmodules and commonjs build of the package in lib/mjs and lib/cjs respectively.
A downside is that it increases the size of the tarball hosted on npm but the upside is that there is probably less configuration etc required by users and the increased package size doesn't really affect them as unused code wouldn't get bundled into their applications anyway.
I haven't thoroughly tested this yet which is why it's a draft, but happy to over the next few days if it'd be useful.
TODO