coinbase / onchainkit

React components and TypeScript utilities to help you build top-tier onchain apps.
https://onchainkit.xyz
MIT License
426 stars 78 forks source link

feat: migrate from packemon to tsup #691

Open roushou opened 2 weeks ago

roushou commented 2 weeks ago

This PR uses tsup for bundling instead of packemon. This should fix the bundling issues I came across recently.

TBH I have no knowledge of packemon so there might be easier solutions, I'm just introducing a tool that does the job fine for me, and maybe it will here too. An objective point though is that tsup is widely used, should be much more battle-tested and have more resources available.

tsup supports:

It also keeps use client directives so this should resolve the issue related to server components. There's a caveat though where code splitting might interfere and not keep directives in chunk files.

Two solutions for that:

The second one is better but it will define all components as client components. Generally this is not ideal but this is fine for OnchainKit since everything is done on the client.

Another thing to consider, dev dependencies are inlined when bundling and not dependencies. So if you want OnchainKit to work without users having to install an external lib then you should put it in dev dependencies.

kyhyco commented 1 week ago

@roushou My preference is to keep using packemon. It's built by one of our past coworker and we use it internally everywhere.

If packemon doesn't work, we can push updates upstream to packemon and the maintainer is very active. (And this would benefit our entire company)

If we need further help on packemon, we can internally pull our packemon experts to help out as well.

Overall, I don't see a huge benefit of replacing packemon with tsup at the moment.


For tree-shaking, we can use knip to remove any unused code. Works super well.


Another thing to consider, dev dependencies are inlined when bundling and not dependencies. So if you want OnchainKit to work without users having to install an external lib then you should put it in dev dependencies.

We should discuss this one further. I too think that viem, wagmi, etc. should be a dev dependencies. There could be a potential conflict between version that the user is using vs what we bundle with.

Zizzamia commented 1 week ago

@kyhyco before introducing any breaking changes, please wait my +1

roushou commented 1 week ago

Another thing to consider, dev dependencies are inlined when bundling and not dependencies. So if you want OnchainKit to work without users having to install an external lib then you should put it in dev dependencies.

For clarity, I was referring to the case where tsup is used for bundling. AFAIK other bundlers (esbuild, rollup, ...) work the other way by inlining dependencies instead of dev dependencies.

We should discuss this one further. I too think that viem, wagmi, etc. should be a dev dependencies. There could be a potential conflict between version that the user is using vs what we bundle with.

IMO it would be better to not inline Viem/Wagmi/... but declare them as peer dependencies. Given the wide range of features those libraries cover it might be hard to cover them all so users might still reach out to them and cause conflicts.

roushou commented 1 week ago

Makes sense to still use packemon. Letting this PR open in case you change your mind. Otherwise feel free to close it.