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.

Sign up for GitHub

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

Jump to bottom

Avoid the assumption of Buffer in browser envs #3452

Merged
merged 8 commits into from Mar 24, 2020
Merged

Avoid the assumption of Buffer in browser envs #3452

Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6e59b6c
Avoid the assumption of Buffer in browser envs
JoviDeCroock Mar 21, 2020
8fda7a9
add comment and one more check for findExistingAssetFileNameWithSource
JoviDeCroock Mar 21, 2020
f4af9ab
remove lockfile
JoviDeCroock Mar 21, 2020
b293eef
correct browser check
JoviDeCroock Mar 21, 2020
8bb40e7
expand browser check
JoviDeCroock Mar 21, 2020
e51e689
Merge branch 'patch-1' of github.com:JoviDeCroock/rollup into patch-1
JoviDeCroock Mar 21, 2020
1a831e5
Make Uint8Array work in browser and add a test
lukastaegert Mar 24, 2020
6614496
Merge branch 'master' into patch-1
lukastaegert Mar 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/utils/FileEmitter.ts
View file
Open in desktop
Expand Up @@ -24,6 +24,8 @@ interface OutputSpecificFileData {
bundle: OutputBundleWithPlaceholders;
}

const isBrowser = typeof window !== 'undefined' && typeof document !== 'undefined';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the motivation but I am not really happy about connecting the presence of Buffer with the presence of some completely unrelated browser globals. A simple fix would be to just check for the presence of Buffer, but there is no reason this cannot work in the browser. Except for equality and type checking, we are not using any more Buffer specific features. I will add some changes and a test.


function generateAssetFileName(
name: string | undefined,
source: string | Buffer,
Expand Down Expand Up @@ -110,7 +112,8 @@ function getValidSource(
emittedFile: { fileName?: string; name?: string },
fileReferenceId: string | null
): string | Buffer {
if (typeof source !== 'string' && !Buffer.isBuffer(source)) {
// Since buffer isn't available in browser and the source isn't a string this should throw.
if (typeof source !== 'string' && (isBrowser || !Buffer.isBuffer(source))) {
const assetName = emittedFile.fileName || emittedFile.name || fileReferenceId;
return error(
errFailedValidation(
Expand Down Expand Up @@ -349,7 +352,7 @@ export class FileEmitter {
const outputFile = bundle[fileName];
if (
outputFile.type === 'asset' &&
(Buffer.isBuffer(source) && Buffer.isBuffer(outputFile.source)
(!isBrowser && Buffer.isBuffer(source) && Buffer.isBuffer(outputFile.source)
? source.equals(outputFile.source)
: source === outputFile.source)
)
Expand Down