vercel / next.js

The React Framework
https://nextjs.org
MIT License
125.92k stars 26.87k forks source link

Unexpected bundle Buffer ployfill into client chunks #40178

Open chentsulin opened 2 years ago

chentsulin commented 2 years ago

Verify canary release

Provide environment information

Operating System: Platform: darwin Arch: arm64 Version: Darwin Kernel Version 21.1.0: Wed Oct 13 17:33:24 PDT 2021; root:xnu-8019.41.5~1/RELEASE_ARM64_T8101 Binaries: Node: 16.15.1 npm: 8.11.0 Yarn: 1.22.18 pnpm: N/A Relevant packages: next: 12.2.5 eslint-config-next: 12.2.5 react: 18.2.0 react-dom: 18.2.0

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

A typeof Buffer check, for example:

const isNode = typeof Buffer !== 'undefined';
console.log({ isNode });

It causes a full Buffer ployfill (22.51Kb) to be put into the client bundle.

Expected Behavior

A typeof Buffer check should just return 'undefined' in browsers instead.

Link to reproduction

https://github.com/chentsulin/nextjs-issue-40178-bundle-client-buffer

To Reproduce

Add the following lines to the pages/index.js file:

const isNode = typeof Buffer !== 'undefined';
console.log({ isNode });
SukkaW commented 2 years ago

It is because Next.js can not identify if you are actually using the Buffer or not.

The proper way of detecting non-browser should be typeof window === 'undefined'.

chentsulin commented 2 years ago

That just a simple way to demonstrate the problem.

In the real case, we use sindresorhus/serialize-error in our app and our bundle size unexpectedly gained 22.51Kb due to these lines in the library:

https://github.com/sindresorhus/serialize-error/blob/a212a8c3902fa1ff1fdef8f7625dd0cc6d6e89a1/index.js#L97-L100

SukkaW commented 2 years ago

@chentsulin It seems that the serialize-error should only be used on the server-side, not to use on the client-side.

chentsulin commented 2 years ago

I believe serialize-error is designed to be used both in browser and server. It handles DOMException as well: https://github.com/sindresorhus/serialize-error/blob/a212a8c3902fa1ff1fdef8f7625dd0cc6d6e89a1/error-constructors.js#L10-L11

If it is designed to be used only on the server-side, it doesn't need to check Buffer exists or not at the first place.

And the weekly downloads of this package almost hits 5M. A lot of people including us already use it with redux ref: https://github.com/reduxjs/redux-toolkit/issues/2392

chentsulin commented 2 years ago

I found a ProvidePlugin that may cause this problem:

https://github.com/vercel/next.js/blob/3466862d9dc9c8bb3131712134d38757b918d1c0/packages/next/build/webpack-config.ts#L1664-L1666

Fonger commented 2 years ago

The comment indicates that the Buffer polyfill is for getInlineScriptSource however it is only used by next.js at server side. (exported at Next/Document)

https://github.com/vercel/next.js/blob/ca807d66f90e9af76e87706c029176d6497d1513/packages/next/pages/_document.tsx#L923

IMO it's not necessary to polyfill Buffer for server-side code because Buffer had been available since node.js 0.10.x ref

If this polyfill is intended for edge runtime, I think we should find a way to fix this issue for client side.

gustaveWPM commented 6 months ago

Hello, I think I have a similar issue.

Using Buffer on the client-side results in a ~ +8kB first-load JS. I'm using Next v14.1.4.

What should I do?

EDIT: I will just "Workaround" it since I have a total control of the use of Buffer in my current implementation, and I'm just going to avoid it... Buuuut... This issue is quite problematic imo.

EDIT2: omg, I'm confused. Buffer is not supported by browsers, but ArrayBuffer is. https://developer.mozilla.org/fr/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer

kirstymullen commented 4 months ago

I found a ProvidePlugin that may cause this problem:

https://github.com/vercel/next.js/blob/3466862d9dc9c8bb3131712134d38757b918d1c0/packages/next/build/webpack-config.ts#L1664-L1666

This does seem to be the culprit. I would like to remove Buffer from our client bundle as part of an optimisation strategy.

The only usages in my code base are these functions:

const base64encode = (string: string) => {
    if (typeof window !== `undefined`) {
        return btoa(string)
    } else {
        return Buffer.from(string, `utf8`).toString(`base64`)
    }
}

const base64decode = (string: string) => {
    if (typeof window !== `undefined`) {
        return atob(string)
    } else {
        return Buffer.from(string, `base64`).toString(`utf8`)
    }
}

If I remove the use of Buffer here, then it disappears from my client bundle.

Is there something I'm missing for Next.js to be able to correctly detect usage on the client and not polyfill? I searched discussions and typeof window !== 'undefined' seemed to be the recommended check.

Any help would be appreciated.

gustaveWPM commented 4 months ago

I found a ProvidePlugin that may cause this problem: https://github.com/vercel/next.js/blob/3466862d9dc9c8bb3131712134d38757b918d1c0/packages/next/build/webpack-config.ts#L1664-L1666

This does seem to be the culprit. I would like to remove Buffer from our client bundle as part of an optimisation strategy.

The only usages in my code base are these functions:

const base64encode = (string: string) => {
    if (typeof window !== `undefined`) {
        return btoa(string)
    } else {
        return Buffer.from(string, `utf8`).toString(`base64`)
    }
}

const base64decode = (string: string) => {
    if (typeof window !== `undefined`) {
        return atob(string)
    } else {
        return Buffer.from(string, `base64`).toString(`utf8`)
    }
}

If I remove the use of Buffer here, then it disappears from my client bundle.

Is there something I'm missing for Next.js to be able to correctly detect usage on the client and not polyfill? I searched discussions and typeof window !== 'undefined' seemed to be the recommended check.

Any help would be appreciated.

I got confused by this too.

Buffer is NOT a front-end API. Buffer is a Node.js BACK-END implementation. Then, when you use Buffer in the front-end, Next.js polyfills it because it's required to run on the front-end.

To be the most confusing as possible, the standards are:

Then, it is ArrayBuffer which is native and that will not produce bundle size inflation. Maybe there's also other front-end built-ins to handle buffers, idk.

kirstymullen commented 4 months ago

I found a ProvidePlugin that may cause this problem: https://github.com/vercel/next.js/blob/3466862d9dc9c8bb3131712134d38757b918d1c0/packages/next/build/webpack-config.ts#L1664-L1666

This does seem to be the culprit. I would like to remove Buffer from our client bundle as part of an optimisation strategy. The only usages in my code base are these functions:

const base64encode = (string: string) => {
    if (typeof window !== `undefined`) {
        return btoa(string)
    } else {
        return Buffer.from(string, `utf8`).toString(`base64`)
    }
}

const base64decode = (string: string) => {
    if (typeof window !== `undefined`) {
        return atob(string)
    } else {
        return Buffer.from(string, `base64`).toString(`utf8`)
    }
}

If I remove the use of Buffer here, then it disappears from my client bundle. Is there something I'm missing for Next.js to be able to correctly detect usage on the client and not polyfill? I searched discussions and typeof window !== 'undefined' seemed to be the recommended check. Any help would be appreciated.

I got confused by this too.

Buffer is NOT a front-end API. Buffer is a Node.js BACK-END implementation. Then, when you use Buffer in the front-end, Next.js polyfills it because it's required to run on the front-end.

To be the most confusing as possible, the standards are:

Then, it is ArrayBuffer which is native and that will not produce bundle size inflation. Maybe there's also other front-end built-ins to handle buffers, idk.

I know that Buffer is not available on the front end, which is why I use btoa and atob when the window is available. So, Buffer will never ever be called by client-side code in my application.

The issue here is that it is not possible to opt out of Buffer being polyfilled when you 100% know that it will not be called on the client.

Given that this issue has been open for ages, I'm likely to just switch to the 'legacy' atob and btoa Node.js functions for the server. I don't want an ArrayBuffer as all I'm doing is decoding/encoding a base64 string.

gustaveWPM commented 4 months ago

I found a ProvidePlugin that may cause this problem: https://github.com/vercel/next.js/blob/3466862d9dc9c8bb3131712134d38757b918d1c0/packages/next/build/webpack-config.ts#L1664-L1666

This does seem to be the culprit. I would like to remove Buffer from our client bundle as part of an optimisation strategy. The only usages in my code base are these functions:

const base64encode = (string: string) => {
    if (typeof window !== `undefined`) {
        return btoa(string)
    } else {
        return Buffer.from(string, `utf8`).toString(`base64`)
    }
}

const base64decode = (string: string) => {
    if (typeof window !== `undefined`) {
        return atob(string)
    } else {
        return Buffer.from(string, `base64`).toString(`utf8`)
    }
}

If I remove the use of Buffer here, then it disappears from my client bundle. Is there something I'm missing for Next.js to be able to correctly detect usage on the client and not polyfill? I searched discussions and typeof window !== 'undefined' seemed to be the recommended check. Any help would be appreciated.

I got confused by this too. Buffer is NOT a front-end API. Buffer is a Node.js BACK-END implementation. Then, when you use Buffer in the front-end, Next.js polyfills it because it's required to run on the front-end. To be the most confusing as possible, the standards are:

Then, it is ArrayBuffer which is native and that will not produce bundle size inflation. Maybe there's also other front-end built-ins to handle buffers, idk.

I know that Buffer is not available on the front end, which is why I use btoa and atob when the window is available. So, Buffer will never ever be called by client-side code in my application.

The issue here is that it is not possible to opt out of Buffer being polyfilled when you 100% know that it will not be called on the client.

Given that this issue has been open for ages, I'm likely to just switch to the 'legacy' atob and btoa Node.js functions for the server. I don't want an ArrayBuffer as all I'm doing is decoding/encoding a base64 string.

Hmmm... Maybe typeof window !== "undefined" is not the better way to get the Buffer polyfill opt-out from the bundle size...

Idk if you're using App or Pages Router. On App Router, I'm used to use some use client magics which opt-out nicely the undesired code. Sometimes, I also need to split my files to help Webpack a bit.

If no solution really works, perhaps the most “Ugly” hack to do would be to lazy load the component in some inelegant way, completely fooling Next.js. https://v3.gatsbyjs.com/docs/mdx/importing-and-using-components/#lazy-loading-components

kirstymullen commented 4 months ago

I found a ProvidePlugin that may cause this problem: https://github.com/vercel/next.js/blob/3466862d9dc9c8bb3131712134d38757b918d1c0/packages/next/build/webpack-config.ts#L1664-L1666

This does seem to be the culprit. I would like to remove Buffer from our client bundle as part of an optimisation strategy. The only usages in my code base are these functions:

const base64encode = (string: string) => {
    if (typeof window !== `undefined`) {
        return btoa(string)
    } else {
        return Buffer.from(string, `utf8`).toString(`base64`)
    }
}

const base64decode = (string: string) => {
    if (typeof window !== `undefined`) {
        return atob(string)
    } else {
        return Buffer.from(string, `base64`).toString(`utf8`)
    }
}

If I remove the use of Buffer here, then it disappears from my client bundle. Is there something I'm missing for Next.js to be able to correctly detect usage on the client and not polyfill? I searched discussions and typeof window !== 'undefined' seemed to be the recommended check. Any help would be appreciated.

I got confused by this too. Buffer is NOT a front-end API. Buffer is a Node.js BACK-END implementation. Then, when you use Buffer in the front-end, Next.js polyfills it because it's required to run on the front-end. To be the most confusing as possible, the standards are:

Then, it is ArrayBuffer which is native and that will not produce bundle size inflation. Maybe there's also other front-end built-ins to handle buffers, idk.

I know that Buffer is not available on the front end, which is why I use btoa and atob when the window is available. So, Buffer will never ever be called by client-side code in my application. The issue here is that it is not possible to opt out of Buffer being polyfilled when you 100% know that it will not be called on the client. Given that this issue has been open for ages, I'm likely to just switch to the 'legacy' atob and btoa Node.js functions for the server. I don't want an ArrayBuffer as all I'm doing is decoding/encoding a base64 string.

Hmmm... Maybe typeof window !== "undefined" is not the better way to get the Buffer polyfill opt-out from the bundle size...

Idk if you're using App or Pages Router. On App Router, I'm used to use some use client magics which opt-out nicely the undesired code. Sometimes, I also need to split my files to help Webpack a bit.

If no solution really works, perhaps the most “Ugly” hack to do would be to lazy load the component in some inelegant way, completely fooling Next.js. https://v3.gatsbyjs.com/docs/mdx/importing-and-using-components/#lazy-loading-components

It's the same regardless of whether it's the pages router or app router (I'm in the process of migrating a large application to the app router). I'm just going to switch to the atob and btoa methods that have both server and client implementations.

Workarounds are great, but it'd be better if Next.js allowed proper control over this specific package being polyfilled or not. It's only an issue with Buffer.