infra: make most pkgs tree shakable

[mPaella]

Description

making most of the packages tree shakable

Test plan

u can see the bundle sizes of our apps has been decreased (new is on the right)

tested payments demo nextjs app

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 77ba139c6247b02dfcad58b0319a3c1529a8d508

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
smart-wallet-auth-demo	⬜️ Ignored (Inspect)	Visit Preview		Sep 27, 2024 3:59pm

[AlbertoElias]

If the config is the same, would it be possible to have one in the root and import it from each package?

[alfonso-paella]

For @AlbertoElias to consider during review -- is this test plan sufficient? Could tree-shaking fail in unexpected ways we must test? (e.g. run a couple demo sites or something)

[mPaella]

If the config is the same, would it be possible to have one in the root and import it from each package?

done

[AlbertoElias]

I'm testing it with other apps. So far, in Smarter Wallet, there are some styles that are not being built correctly so I'm looking into it.

@jmderby could you chime in with how twind works to create classes on the fly? The issue comes with classes that are set conditionally during execution such as:

<iframe
    ref={iframeRef}
    src={iframeSrc}
    onLoad={handleIframeLoaded}
    title="Authentication Modal"
    className={classNames(
        "w-full h-[500px] border pt-12 pb-8",
        appearance?.colors?.border
            ? `border-[${appearance.colors.border}]`
            : "border-[#D0D5DD]",
        appearance?.borderRadius ? `rounded-[${appearance.borderRadius}]` : "rounded-2xl",
        appearance?.colors?.background ? `bg-[${appearance.colors.background}]` : "bg-white"
    )}
/>

[jmderby]

I'm testing it with other apps. So far, in Smarter Wallet, there are some styles that are not being built correctly so I'm looking into it.

@jmderby could you chime in with how twind works to create classes on the fly? The issue comes with classes that are set conditionally during execution such as:

<iframe
    ref={iframeRef}
    src={iframeSrc}
    onLoad={handleIframeLoaded}
    title="Authentication Modal"
    className={classNames(
        "w-full h-[500px] border pt-12 pb-8",
        appearance?.colors?.border
            ? `border-[${appearance.colors.border}]`
            : "border-[#D0D5DD]",
        appearance?.borderRadius ? `rounded-[${appearance.borderRadius}]` : "rounded-2xl",
        appearance?.colors?.background ? `bg-[${appearance.colors.background}]` : "bg-white"
    )}
/>

Yeah, I might have to re-evaluate how we're handling the dynamic classes by using something like inline styles. Dynamically setting I guess breaks tailwind rules. https://tailwindcss.com/docs/content-configuration#dynamic-class-names:~:text=Don%E2%80%99t%20use%20props%20to%20build%20class%20names%20dynamically

[mPaella]

yeah, classes like border-[${appearance.colors.border}] are known to not work/be flaky

better to use style object for those

[jmderby]

yeah, classes like border-[${appearance.colors.border}] are known to not work/be flaky

better to use style object for those

Yeah, I'm looking at safelisting (https://tailwindcss.com/docs/content-configuration#safelisting-classes) but it seems like inline might be the most reliable method.

[jmderby]

@maxwellfortney @AlbertoElias do you guys want me to prioritize a PR that replaces dynam classes with inline asap?

[mPaella]

not sure safelisting will even work, but i would just go for styles for simplicity + certainty it will work

want me to prioritize a PR

assuming its currently broken, i think that makes sense

in the context of this pr tho, i think we can say the conditional styles not working is not due to the tree shaking

[AlbertoElias]

in the context of this pr tho, i think we can say the conditional styles not working is not due to the tree shaking

Not due to the tree shaking, but something changes in the build process that breaks this while it does work in main. I'll finish testing other apps and report back

[AlbertoElias]

Looking good. But we should wait to merge this until @jmderby fixes the twind stuff on react-ui or development and testing will be a pain

[AlbertoElias]

@mPaella Implemented #805 so this can now be merged.