Skip to content
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

Reduce bundle size, when using a flag that does not depend on #129

Open
1 task done
kazupon opened this issue Apr 8, 2024 · 2 comments
Open
1 task done

Reduce bundle size, when using a flag that does not depend on #129

kazupon opened this issue Apr 8, 2024 · 2 comments

Comments

@kazupon
Copy link

kazupon commented Apr 8, 2024

Describe the feature

What to Expect

If we use this flag, providers will be bundled, but even if we use a flag that does not depend on providers, such as isWindows, they will still be bundled.

If they do not depend on it, we expect them not to be bundled.

Also, if you use isCI, all providers' code will be bundled. It might be nice to have a way for users to resolve providers themselves by exporting an API that resolves them.

Reproduction repo

https://github.com/kazupon/std-env-repro1

Related

rolldown/rolldown#754 (comment)

Additional information

  • Would you be willing to help implement this feature?
@pi0
Copy link
Member

pi0 commented Apr 8, 2024

Thanks for nice reproduction.

I have made #130 in attempt to see how we can optimize more.

TLDR is, rollup does not trust only sideEffects: true field of package.json, we need to annotate internals calls like new Proxy() constructor even with /* @__PURE__ */ comment to tell rollup tree-shake them and in minified build of std-env esbuilds removes them and ALSO we need to opt-in to an unsafe rollup tree-shake option to even be able to leverage it.

With changes of linked PR and custom unbuild configuration below, you can see isWindows can be tree-shaken only:

import { defineBuildConfig } from 'unbuild'

export default defineBuildConfig({
    entries: [
        // 'src/ci.ts',
        'src/windows.ts'
    ],
    rollup: {
        inlineDependencies: true,
    },
    hooks: {
        "rollup:options"(_ctx, options) {
            options.treeshake = 'smallest'
        }
    }
})

At this point, I really think it is best for the majority of the ecosystem that we keep shipping one minified dist. 1kB extra in CLI bundle is relatively better than doing any other over engineering IMO and many flags even isCI needs provider information which takes main portion of this.

@kazupon
Copy link
Author

kazupon commented Apr 8, 2024

Thank you for your kind reply.

I understand that unjs is also used in other ecosystems, including nuxt.
The effort to remove providers for a 1KB reduction is indeed over-engineering for unjs, as you say.

isCI flag, and other flags that depend on it, can be implemented in an existing another way, so it certainly doesn't make sense to over-engineer it for my use case for unjs.

I appreciate you are making experimenting PR & giving unbuild config :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants