Avoid the assumption of Buffer in browser envs (#3452)

* Avoid the assumption of Buffer in browser envs * add comment and one more check for findExistingAssetFileNameWithSource * remove lockfile * correct browser check * expand browser check * Make Uint8Array work in browser and add a test Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com> Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com>
rollup · Mar 24, 2020 · e9ece1d · e9ece1d
1 parent 399bbf4
commit e9ece1d
Show file tree

Hide file tree

Showing 21 changed files with 235 additions and 85 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -80,7 +80,7 @@
 * Modules that are completely tree-shaken will no longer be listed as part of any chunks in `generateBundle`
 * The `experimentalOptimizeChunks` and `chunkGroupingSize` options have been removed
 * [acorn](https://github.com/acornjs/acorn) plugins can only be used if they accept a passed-in acorn instance instead of importing it themselves. See https://github.com/acornjs/acorn/pull/870#issuecomment-527339830 for what needs to be done to make plugins compatible that do not support this yet (#3391)
-* Emitted chunks now have the TypeScript type `UInt8Array` instead of `Buffer`. A `Buffer` can still be used, though (#3395)
+* Emitted chunks now have the TypeScript type `Uint8Array` instead of `Buffer`. A `Buffer` can still be used, though (#3395)
 * The TypeScript types no longer use ESTree types for AST nodes but a very generic type that does not contain information specific to certain node types (#3395)
 * The signature of the `writeBundle` plugin hook has been changed to match `generateBundle`: The bundle object is now passed as second parameter instead of first and the first parameter is the output options (#3361)
 * The following plugin hooks have been removed:

diff --git a/docs/02-javascript-api.md b/docs/02-javascript-api.md
@@ -29,7 +29,7 @@ async function build() {
       // For assets, this contains
       // {
       //   fileName: string,              // the asset file name
-      //   source: string | UInt8Array    // the asset source
+      //   source: string | Uint8Array    // the asset source
       //   type: 'asset'                  // signifies that this is an asset
       // }
       console.log('Asset', chunkOrAsset);

diff --git a/docs/05-plugin-development.md b/docs/05-plugin-development.md
@@ -282,7 +282,7 @@ Called at the end of `bundle.generate()` or immediately before the files are wri
 // AssetInfo
 {
   fileName: string,
-  source: string | UInt8Array,
+  source: string | Uint8Array,
   type: 'asset',
 }
 
@@ -457,7 +457,7 @@ Emits a new file that is included in the build output and returns a `referenceId
 // EmittedAsset
 {
   type: 'asset',
-  source?: string | UInt8Array,
+  source?: string | Uint8Array,
   name?: string,
   fileName?: string
 }
@@ -526,9 +526,9 @@ Resolve imports to module ids (i.e. file names) using the same plugins that Roll
 
 If you pass `skipSelf: true`, then the `resolveId` hook of the plugin from which `this.resolve` is called will be skipped when resolving.
 
-#### `this.setAssetSource(assetReferenceId: string, source: string | UInt8Array) => void`
+#### `this.setAssetSource(assetReferenceId: string, source: string | Uint8Array) => void`
 
-Set the deferred source of an asset. Note that you can also pass a Node `Buffer` as `source` as it is a sub-class of `UInt8Array`.
+Set the deferred source of an asset. Note that you can also pass a Node `Buffer` as `source` as it is a sub-class of `Uint8Array`.
 
 #### `this.warn(warning: string | RollupWarning, position?: number | { column: number; line: number }) => void`
 
@@ -550,7 +550,7 @@ The `position` argument is a character index where the warning was raised. If pr
 
 ☢️ These context utility functions have been deprecated and may be removed in a future Rollup version.
 
-- `this.emitAsset(assetName: string, source: string) => string` - _**Use [`this.emitFile`](guide/en/#thisemitfileemittedfile-emittedchunk--emittedasset--string)**_ - Emits a custom file that is included in the build output, returning an `assetReferenceId` that can be used to reference the emitted file. You can defer setting the source if you provide it later via [`this.setAssetSource(assetReferenceId, source)`](guide/en/#thissetassetsourceassetreferenceid-string-source-string--uint8array--void). A string or `UInt8Array`/`Buffer` source must be set for each asset through either method or an error will be thrown on generate completion.
+- `this.emitAsset(assetName: string, source: string) => string` - _**Use [`this.emitFile`](guide/en/#thisemitfileemittedfile-emittedchunk--emittedasset--string)**_ - Emits a custom file that is included in the build output, returning an `assetReferenceId` that can be used to reference the emitted file. You can defer setting the source if you provide it later via [`this.setAssetSource(assetReferenceId, source)`](guide/en/#thissetassetsourceassetreferenceid-string-source-string--uint8array--void). A string or `Uint8Array`/`Buffer` source must be set for each asset through either method or an error will be thrown on generate completion.
 
   Emitted assets will follow the [`output.assetFileNames`](guide/en/#outputassetfilenames) naming scheme. You can reference the URL of the file in any code returned by a [`load`](guide/en/#load) or [`transform`](guide/en/#transform) plugin hook via `import.meta.ROLLUP_ASSET_URL_assetReferenceId`.
 

diff --git a/src/utils/FileEmitter.ts b/src/utils/FileEmitter.ts
@@ -26,7 +26,7 @@ interface OutputSpecificFileData {
 
 function generateAssetFileName(
 	name: string | undefined,
-	source: string | Buffer,
+	source: string | Uint8Array,
 	output: OutputSpecificFileData
 ): string {
 	const emittedName = name || 'asset';
@@ -68,7 +68,7 @@ interface ConsumedChunk {
 interface ConsumedAsset {
 	fileName: string | undefined;
 	name: string | undefined;
-	source: string | Buffer | undefined;
+	source: string | Uint8Array | undefined;
 	type: 'asset';
 }
 
@@ -109,14 +109,14 @@ function getValidSource(
 	source: unknown,
 	emittedFile: { fileName?: string; name?: string },
 	fileReferenceId: string | null
-): string | Buffer {
-	if (typeof source !== 'string' && !Buffer.isBuffer(source)) {
+): string | Uint8Array {
+	if (!(typeof source === 'string' || source instanceof Uint8Array)) {
 		const assetName = emittedFile.fileName || emittedFile.name || fileReferenceId;
 		return error(
 			errFailedValidation(
 				`Could not set source for ${
 					typeof assetName === 'string' ? `asset "${assetName}"` : 'unnamed asset'
-				}, asset source needs to be a string of Buffer.`
+				}, asset source needs to be a string, Uint8Array or Buffer.`
 			)
 		);
 	}
@@ -313,13 +313,13 @@ export class FileEmitter {
 
 	private finalizeAsset(
 		consumedFile: ConsumedFile,
-		source: string | Buffer,
+		source: string | Uint8Array,
 		referenceId: string,
 		output: OutputSpecificFileData
 	): void {
 		const fileName =
 			consumedFile.fileName ||
-			this.findExistingAssetFileNameWithSource(output.bundle, source) ||
+			findExistingAssetFileNameWithSource(output.bundle, source) ||
 			generateAssetFileName(consumedFile.name, source, output);
 
 		// We must not modify the original assets to avoid interaction between outputs
@@ -340,21 +340,39 @@ export class FileEmitter {
 			type: 'asset'
 		};
 	}
+}
 
-	private findExistingAssetFileNameWithSource(
-		bundle: OutputBundleWithPlaceholders,
-		source: string | Buffer
-	): string | null {
-		for (const fileName of Object.keys(bundle)) {
-			const outputFile = bundle[fileName];
-			if (
-				outputFile.type === 'asset' &&
-				(Buffer.isBuffer(source) && Buffer.isBuffer(outputFile.source)
-					? source.equals(outputFile.source)
-					: source === outputFile.source)
-			)
-				return fileName;
+function findExistingAssetFileNameWithSource(
+	bundle: OutputBundleWithPlaceholders,
+	source: string | Uint8Array
+): string | null {
+	for (const fileName of Object.keys(bundle)) {
+		const outputFile = bundle[fileName];
+		if (outputFile.type === 'asset' && areSourcesEqual(source, outputFile.source)) return fileName;
+	}
+	return null;
+}
+
+function areSourcesEqual(
+	sourceA: string | Uint8Array | Buffer,
+	sourceB: string | Uint8Array | Buffer
+): boolean {
+	if (typeof sourceA === 'string') {
+		return sourceA === sourceB;
+	}
+	if (typeof sourceB === 'string') {
+		return false;
+	}
+	if ('equals' in sourceA) {
+		return sourceA.equals(sourceB);
+	}
+	if (sourceA.length !== sourceB.length) {
+		return false;
+	}
+	for (let index = 0; index < sourceA.length; index++) {
+		if (sourceA[index] !== sourceB[index]) {
+			return false;
 		}
-		return null;
 	}
+	return true;
 }
diff --git a/test/form/index.js b/test/form/index.js
@@ -12,12 +12,13 @@ runTestSuiteWithSamples('form', path.resolve(__dirname, 'samples'), (dir, config
 	(config.skip ? itOrDescribe.skip : config.solo ? itOrDescribe.only : itOrDescribe)(
 		path.basename(dir) + ': ' + config.description,
 		() => {
-			let promise;
-			const runRollupTest = (inputFile, bundleFile, defaultFormat) => {
+			let bundle;
+			const runRollupTest = async (inputFile, bundleFile, defaultFormat) => {
+				if (config.before) config.before();
 				process.chdir(dir);
-				return (
-					promise ||
-					(promise = rollup.rollup(
+				bundle =
+					bundle ||
+					(await rollup.rollup(
 						extend(
 							{
 								input: dir + '/main.js',
@@ -35,21 +36,20 @@ runTestSuiteWithSamples('form', path.resolve(__dirname, 'samples'), (dir, config
 							},
 							config.options || {}
 						)
-					))
-				).then(bundle =>
-					generateAndTestBundle(
-						bundle,
-						extend(
-							{
-								file: inputFile,
-								format: defaultFormat
-							},
-							(config.options || {}).output || {}
-						),
-						bundleFile,
-						config
-					)
+					));
+				await generateAndTestBundle(
+					bundle,
+					extend(
+						{
+							file: inputFile,
+							format: defaultFormat
+						},
+						(config.options || {}).output || {}
+					),
+					bundleFile,
+					config
 				);
+				if (config.after) config.after();
 			};
 
 			if (isSingleFormatTest) {
@@ -69,42 +69,41 @@ runTestSuiteWithSamples('form', path.resolve(__dirname, 'samples'), (dir, config
 	);
 });
 
-function generateAndTestBundle(bundle, outputOptions, expectedFile, { show }) {
-	return bundle.write(outputOptions).then(() => {
-		const actualCode = normaliseOutput(sander.readFileSync(outputOptions.file));
-		let expectedCode;
-		let actualMap;
-		let expectedMap;
+async function generateAndTestBundle(bundle, outputOptions, expectedFile, { show }) {
+	await bundle.write(outputOptions);
+	const actualCode = normaliseOutput(sander.readFileSync(outputOptions.file));
+	let expectedCode;
+	let actualMap;
+	let expectedMap;
 
-		try {
-			expectedCode = normaliseOutput(sander.readFileSync(expectedFile));
-		} catch (err) {
-			expectedCode = 'missing file';
-		}
+	try {
+		expectedCode = normaliseOutput(sander.readFileSync(expectedFile));
+	} catch (err) {
+		expectedCode = 'missing file';
+	}
 
-		try {
-			actualMap = JSON.parse(sander.readFileSync(outputOptions.file + '.map').toString());
-			actualMap.sourcesContent = actualMap.sourcesContent
-				? actualMap.sourcesContent.map(normaliseOutput)
-				: null;
-		} catch (err) {
-			assert.equal(err.code, 'ENOENT');
-		}
+	try {
+		actualMap = JSON.parse(sander.readFileSync(outputOptions.file + '.map').toString());
+		actualMap.sourcesContent = actualMap.sourcesContent
+			? actualMap.sourcesContent.map(normaliseOutput)
+			: null;
+	} catch (err) {
+		assert.equal(err.code, 'ENOENT');
+	}
 
-		try {
-			expectedMap = JSON.parse(sander.readFileSync(expectedFile + '.map').toString());
-			expectedMap.sourcesContent = actualMap.sourcesContent
-				? expectedMap.sourcesContent.map(normaliseOutput)
-				: null;
-		} catch (err) {
-			assert.equal(err.code, 'ENOENT');
-		}
+	try {
+		expectedMap = JSON.parse(sander.readFileSync(expectedFile + '.map').toString());
+		expectedMap.sourcesContent = actualMap.sourcesContent
+			? expectedMap.sourcesContent.map(normaliseOutput)
+			: null;
+	} catch (err) {
+		assert.equal(err.code, 'ENOENT');
+	}
 
-		if (show) {
-			console.log(actualCode + '\n\n\n');
-		}
+	if (show) {
+		console.log(actualCode + '\n\n\n');
+	}
 
-		assert.equal(actualCode, expectedCode);
-		assert.deepEqual(actualMap, expectedMap);
-	});
+	assert.equal(actualCode, expectedCode);
+	assert.deepEqual(actualMap, expectedMap);
 }
diff --git a/test/form/samples/emit-uint8array-no-buffer/_config.js b/test/form/samples/emit-uint8array-no-buffer/_config.js
@@ -0,0 +1,29 @@
+const Buffer = global.Buffer;
+
+module.exports = {
+	description:
+		'supports emitting assets as Uint8Arrays when Buffer is not available with deduplication',
+	before() {
+		delete global.Buffer;
+	},
+	after() {
+		global.Buffer = Buffer;
+	},
+	options: {
+		plugins: {
+			resolveId(id) {
+				if (id.startsWith('asset')) {
+					return id;
+				}
+			},
+			load(id) {
+				if (id.startsWith('asset')) {
+					return `export default import.meta.ROLLUP_FILE_URL_${this.emitFile({
+						type: 'asset',
+						source: Uint8Array.from(Array.from(id.slice(0, -1)).map(char => char.charCodeAt(0)))
+					})};`;
+				}
+			}
+		}
+	}
+};
diff --git a/test/form/samples/emit-uint8array-no-buffer/_expected/amd.js b/test/form/samples/emit-uint8array-no-buffer/_expected/amd.js
@@ -0,0 +1,15 @@
+define(['require'], function (require) { 'use strict';
+
+	var asset1a = new URL(require.toUrl('./assets/asset-dc5cb674'), document.baseURI).href;
+
+	var asset1b = new URL(require.toUrl('./assets/asset-dc5cb674'), document.baseURI).href;
+
+	var asset2a = new URL(require.toUrl('./assets/asset-52cbd095'), document.baseURI).href;
+
+	var asset2b = new URL(require.toUrl('./assets/asset-52cbd095'), document.baseURI).href;
+
+	var asset99a = new URL(require.toUrl('./assets/asset-c568a840'), document.baseURI).href;
+
+	console.log(asset1a, asset1b, asset2a, asset2b, asset99a);
+
+});
diff --git a/test/form/samples/emit-uint8array-no-buffer/_expected/assets/asset-52cbd095 b/test/form/samples/emit-uint8array-no-buffer/_expected/assets/asset-52cbd095
@@ -0,0 +1 @@
+asset2
diff --git a/test/form/samples/emit-uint8array-no-buffer/_expected/assets/asset-c568a840 b/test/form/samples/emit-uint8array-no-buffer/_expected/assets/asset-c568a840
@@ -0,0 +1 @@
+asset99
diff --git a/test/form/samples/emit-uint8array-no-buffer/_expected/assets/asset-dc5cb674 b/test/form/samples/emit-uint8array-no-buffer/_expected/assets/asset-dc5cb674
@@ -0,0 +1 @@
+asset1
diff --git a/test/form/samples/emit-uint8array-no-buffer/_expected/cjs.js b/test/form/samples/emit-uint8array-no-buffer/_expected/cjs.js
@@ -0,0 +1,13 @@
+'use strict';
+
+var asset1a = (typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __dirname + '/assets/asset-dc5cb674').href : new URL('assets/asset-dc5cb674', document.currentScript && document.currentScript.src || document.baseURI).href);
+
+var asset1b = (typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __dirname + '/assets/asset-dc5cb674').href : new URL('assets/asset-dc5cb674', document.currentScript && document.currentScript.src || document.baseURI).href);
+
+var asset2a = (typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __dirname + '/assets/asset-52cbd095').href : new URL('assets/asset-52cbd095', document.currentScript && document.currentScript.src || document.baseURI).href);
+
+var asset2b = (typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __dirname + '/assets/asset-52cbd095').href : new URL('assets/asset-52cbd095', document.currentScript && document.currentScript.src || document.baseURI).href);
+
+var asset99a = (typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __dirname + '/assets/asset-c568a840').href : new URL('assets/asset-c568a840', document.currentScript && document.currentScript.src || document.baseURI).href);
+
+console.log(asset1a, asset1b, asset2a, asset2b, asset99a);
diff --git a/test/form/samples/emit-uint8array-no-buffer/_expected/es.js b/test/form/samples/emit-uint8array-no-buffer/_expected/es.js
@@ -0,0 +1,11 @@
+var asset1a = new URL('assets/asset-dc5cb674', import.meta.url).href;
+
+var asset1b = new URL('assets/asset-dc5cb674', import.meta.url).href;
+
+var asset2a = new URL('assets/asset-52cbd095', import.meta.url).href;
+
+var asset2b = new URL('assets/asset-52cbd095', import.meta.url).href;
+
+var asset99a = new URL('assets/asset-c568a840', import.meta.url).href;
+
+console.log(asset1a, asset1b, asset2a, asset2b, asset99a);
diff --git a/test/form/samples/emit-uint8array-no-buffer/_expected/iife.js b/test/form/samples/emit-uint8array-no-buffer/_expected/iife.js
@@ -0,0 +1,16 @@
+(function () {
+	'use strict';
+
+	var asset1a = new URL('assets/asset-dc5cb674', document.currentScript && document.currentScript.src || document.baseURI).href;
+
+	var asset1b = new URL('assets/asset-dc5cb674', document.currentScript && document.currentScript.src || document.baseURI).href;
+
+	var asset2a = new URL('assets/asset-52cbd095', document.currentScript && document.currentScript.src || document.baseURI).href;
+
+	var asset2b = new URL('assets/asset-52cbd095', document.currentScript && document.currentScript.src || document.baseURI).href;
+
+	var asset99a = new URL('assets/asset-c568a840', document.currentScript && document.currentScript.src || document.baseURI).href;
+
+	console.log(asset1a, asset1b, asset2a, asset2b, asset99a);
+
+}());