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

Refactor use async control flow #3053

Merged
merged 9 commits into from Aug 15, 2019
Merged

Refactor use async control flow #3053

Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ad9c824
Minor refactoring: Remove one try-catch
KSXGitHub Aug 12, 2019
2b06ebc
Use async control flow in place of .then() chain
KSXGitHub Aug 12, 2019
c783380
Restore some .then()s to fix the tests
KSXGitHub Aug 12, 2019
7fe4777
Pass err from 'buildStart' hook to 'buildEnd'
KSXGitHub Aug 12, 2019
4fcacdf
Convert another promise chain into async try/catch
KSXGitHub Aug 13, 2019
4aaf416
Convert another promise chain to async control flow
KSXGitHub Aug 13, 2019
1d7487e
Merge remote-tracking branch 'upstream/master' into refactor-use-asyn…
lukastaegert Aug 14, 2019
5f52d8e
Merge remote-tracking branch 'upstream/master' into refactor-use-asyn…
lukastaegert Aug 14, 2019
02b222c
Merge branch 'master' into refactor-use-async-control-flow
shellscape Aug 14, 2019
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
332 changes: 159 additions & 173 deletions src/rollup/index.ts
View file
Open in desktop
Expand Up @@ -174,184 +174,170 @@ function assignChunksToBundle(
return outputBundle as OutputBundle;
}

export default function rollup(rawInputOptions: GenericConfigObject): Promise<RollupBuild> {
try {
const inputOptions = getInputOptions(rawInputOptions);
initialiseTimers(inputOptions);

const graph = new Graph(inputOptions, curWatcher);
curWatcher = undefined as any;

// remove the cache option from the memory after graph creation (cache is not used anymore)
const useCache = rawInputOptions.cache !== false;
delete inputOptions.cache;
delete rawInputOptions.cache;

timeStart('BUILD', 1);

return graph.pluginDriver
.hookParallel('buildStart', [inputOptions])
.then(() =>
graph.build(
inputOptions.input as string | string[] | Record<string, string>,
inputOptions.manualChunks,
inputOptions.inlineDynamicImports as boolean
)
)
.then(
chunks => graph.pluginDriver.hookParallel('buildEnd', []).then(() => chunks),
err =>
graph.pluginDriver.hookParallel('buildEnd', [err]).then(() => {
throw err;
})
export default async function rollup(rawInputOptions: GenericConfigObject): Promise<RollupBuild> {
const inputOptions = getInputOptions(rawInputOptions);
initialiseTimers(inputOptions);

const graph = new Graph(inputOptions, curWatcher);
curWatcher = undefined as any;

// remove the cache option from the memory after graph creation (cache is not used anymore)
const useCache = rawInputOptions.cache !== false;
delete inputOptions.cache;
delete rawInputOptions.cache;

timeStart('BUILD', 1);

const chunks = await graph.pluginDriver
.hookParallel('buildStart', [inputOptions])
.then(() =>
graph.build(
inputOptions.input as string | string[] | Record<string, string>,
inputOptions.manualChunks,
inputOptions.inlineDynamicImports as boolean
)
.then(chunks => {
timeEnd('BUILD', 1);

// ensure we only do one optimization pass per build
let optimized = false;

function getOutputOptions(rawOutputOptions: GenericConfigObject) {
return normalizeOutputOptions(
inputOptions as GenericConfigObject,
rawOutputOptions,
chunks.length > 1,
graph.pluginDriver
);
}
)
.then(
Copy link
Member

Choose a reason for hiding this comment

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

Moving to a more async-await based flow is definitely something we want but this is not an improvement now that the different styles are mixed and we have async functions inside Promise chains inside an async function. If you complete the refactoring to use a try-catch block to catch the error and get rid of the remaining Promise chains in this section, I could see this merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lukastaegert You want this?

	let chunks: Chunk[]

	try {
		await graph.pluginDriver.hookParallel('buildStart', [inputOptions]);
		chunks = await graph.build(
			inputOptions.input as string | string[] | Record<string, string>,
			inputOptions.manualChunks,
			inputOptions.inlineDynamicImports as boolean
		);
	} catch (err) {
		await graph.pluginDriver.hookParallel('buildEnd', [err]);
		throw err;
	}

	await graph.pluginDriver.hookParallel('buildEnd', []);

For reference, this is the original:

	const chunks = await graph.pluginDriver
		.hookParallel('buildStart', [inputOptions])
		.then(() =>
			graph.build(
				inputOptions.input as string | string[] | Record<string, string>,
				inputOptions.manualChunks,
				inputOptions.inlineDynamicImports as boolean
			)
		)
		.then(
			async chunks => {
				await graph.pluginDriver.hookParallel('buildEnd', []);
				return chunks;
			},
			async err => {
				await graph.pluginDriver.hookParallel('buildEnd', [err]);
				throw err;
			}
		);

Copy link
Member

Choose a reason for hiding this comment

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

Exactly 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I already done that (4fcacdf).

async chunks => {
await graph.pluginDriver.hookParallel('buildEnd', []);
return chunks;
},
async err => {
await graph.pluginDriver.hookParallel('buildEnd', [err]);
throw err;
}
);

async function generate(
outputOptions: OutputOptions,
isWrite: boolean
): Promise<OutputBundle> {
timeStart('GENERATE', 1);

const assetFileNames = outputOptions.assetFileNames || 'assets/[name]-[hash][extname]';
const outputBundleWithPlaceholders: OutputBundleWithPlaceholders = Object.create(null);
let outputBundle;
const inputBase = commondir(getAbsoluteEntryModulePaths(chunks));
graph.pluginDriver.startOutput(outputBundleWithPlaceholders, assetFileNames);

try {
await graph.pluginDriver.hookParallel('renderStart', []);
const addons = await createAddons(graph, outputOptions);
for (const chunk of chunks) {
if (!inputOptions.preserveModules) chunk.generateInternalExports(outputOptions);
if (chunk.facadeModule && chunk.facadeModule.isEntryPoint)
chunk.exportMode = getExportMode(chunk, outputOptions);
}
for (const chunk of chunks) {
chunk.preRender(outputOptions, inputBase);
}
if (!optimized && inputOptions.experimentalOptimizeChunks) {
optimizeChunks(
chunks,
outputOptions,
inputOptions.chunkGroupingSize as number,
inputBase
);
optimized = true;
}
assignChunkIds(
chunks,
inputOptions,
outputOptions,
inputBase,
addons,
outputBundleWithPlaceholders
);
outputBundle = assignChunksToBundle(chunks, outputBundleWithPlaceholders);

await Promise.all(
chunks.map(chunk => {
const outputChunk = outputBundleWithPlaceholders[chunk.id as string] as OutputChunk;
return chunk.render(outputOptions, addons, outputChunk).then(rendered => {
outputChunk.code = rendered.code;
outputChunk.map = rendered.map;

return graph.pluginDriver.hookParallel('ongenerate', [
{ bundle: outputChunk, ...outputOptions },
outputChunk
]);
});
})
);
} catch (error) {
await graph.pluginDriver.hookParallel('renderError', [error]);
throw error;
}
await graph.pluginDriver.hookSeq('generateBundle', [
outputOptions,
outputBundle,
isWrite
]);
graph.pluginDriver.finaliseAssets();

timeEnd('GENERATE', 1);
return outputBundle;
}
timeEnd('BUILD', 1);

// ensure we only do one optimization pass per build
let optimized = false;

function getOutputOptions(rawOutputOptions: GenericConfigObject) {
return normalizeOutputOptions(
inputOptions as GenericConfigObject,
rawOutputOptions,
chunks.length > 1,
graph.pluginDriver
);
}

async function generate(outputOptions: OutputOptions, isWrite: boolean): Promise<OutputBundle> {
timeStart('GENERATE', 1);

const assetFileNames = outputOptions.assetFileNames || 'assets/[name]-[hash][extname]';
const outputBundleWithPlaceholders: OutputBundleWithPlaceholders = Object.create(null);
let outputBundle;
const inputBase = commondir(getAbsoluteEntryModulePaths(chunks));
graph.pluginDriver.startOutput(outputBundleWithPlaceholders, assetFileNames);

try {
await graph.pluginDriver.hookParallel('renderStart', []);
const addons = await createAddons(graph, outputOptions);
for (const chunk of chunks) {
if (!inputOptions.preserveModules) chunk.generateInternalExports(outputOptions);
if (chunk.facadeModule && chunk.facadeModule.isEntryPoint)
chunk.exportMode = getExportMode(chunk, outputOptions);
}
for (const chunk of chunks) {
chunk.preRender(outputOptions, inputBase);
}
if (!optimized && inputOptions.experimentalOptimizeChunks) {
optimizeChunks(chunks, outputOptions, inputOptions.chunkGroupingSize as number, inputBase);
optimized = true;
}
assignChunkIds(
chunks,
inputOptions,
outputOptions,
inputBase,
addons,
outputBundleWithPlaceholders
);
outputBundle = assignChunksToBundle(chunks, outputBundleWithPlaceholders);

await Promise.all(
chunks.map(chunk => {
const outputChunk = outputBundleWithPlaceholders[chunk.id as string] as OutputChunk;
return chunk.render(outputOptions, addons, outputChunk).then(rendered => {
outputChunk.code = rendered.code;
outputChunk.map = rendered.map;

return graph.pluginDriver.hookParallel('ongenerate', [
{ bundle: outputChunk, ...outputOptions },
outputChunk
]);
});
})
);
} catch (error) {
await graph.pluginDriver.hookParallel('renderError', [error]);
throw error;
}
await graph.pluginDriver.hookSeq('generateBundle', [outputOptions, outputBundle, isWrite]);
graph.pluginDriver.finaliseAssets();

const cache = useCache ? graph.getCache() : undefined;
const result: RollupBuild = {
cache: cache as RollupCache,
generate: ((rawOutputOptions: GenericConfigObject) => {
const promise = generate(getOutputOptions(rawOutputOptions), false).then(result =>
createOutput(result)
);
Object.defineProperty(promise, 'code', throwAsyncGenerateError);
Object.defineProperty(promise, 'map', throwAsyncGenerateError);
return promise;
}) as any,
watchFiles: Object.keys(graph.watchFiles),
write: ((rawOutputOptions: GenericConfigObject) => {
const outputOptions = getOutputOptions(rawOutputOptions);
if (!outputOptions.dir && !outputOptions.file) {
error({
code: 'MISSING_OPTION',
message: 'You must specify "output.file" or "output.dir" for the build.'
});
}
return generate(outputOptions, true).then(bundle => {
let chunkCnt = 0;
for (const fileName of Object.keys(bundle)) {
const file = bundle[fileName];
if ((file as OutputAsset).isAsset) continue;
chunkCnt++;
if (chunkCnt > 1) break;
}
if (chunkCnt > 1) {
if (outputOptions.sourcemapFile)
error({
code: 'INVALID_OPTION',
message: '"output.sourcemapFile" is only supported for single-file builds.'
});
if (typeof outputOptions.file === 'string')
error({
code: 'INVALID_OPTION',
message:
'When building multiple chunks, the "output.dir" option must be used, not "output.file".' +
(typeof inputOptions.input !== 'string' ||
inputOptions.inlineDynamicImports === true
? ''
: ' To inline dynamic imports, set the "inlineDynamicImports" option.')
});
}
return Promise.all(
Object.keys(bundle).map(chunkId =>
writeOutputFile(graph, result, bundle[chunkId], outputOptions)
)
)
.then(() => graph.pluginDriver.hookParallel('writeBundle', [bundle]))
.then(() => createOutput(bundle));
timeEnd('GENERATE', 1);
return outputBundle;
}

const cache = useCache ? graph.getCache() : undefined;
const result: RollupBuild = {
cache: cache as RollupCache,
generate: ((rawOutputOptions: GenericConfigObject) => {
const promise = generate(getOutputOptions(rawOutputOptions), false).then(result =>
createOutput(result)
);
Object.defineProperty(promise, 'code', throwAsyncGenerateError);
Object.defineProperty(promise, 'map', throwAsyncGenerateError);
return promise;
}) as any,
watchFiles: Object.keys(graph.watchFiles),
write: ((rawOutputOptions: GenericConfigObject) => {
const outputOptions = getOutputOptions(rawOutputOptions);
if (!outputOptions.dir && !outputOptions.file) {
error({
code: 'MISSING_OPTION',
message: 'You must specify "output.file" or "output.dir" for the build.'
});
}
return generate(outputOptions, true).then(bundle => {
let chunkCnt = 0;
for (const fileName of Object.keys(bundle)) {
const file = bundle[fileName];
if ((file as OutputAsset).isAsset) continue;
chunkCnt++;
if (chunkCnt > 1) break;
}
if (chunkCnt > 1) {
if (outputOptions.sourcemapFile)
error({
code: 'INVALID_OPTION',
message: '"output.sourcemapFile" is only supported for single-file builds.'
});
if (typeof outputOptions.file === 'string')
error({
code: 'INVALID_OPTION',
message:
'When building multiple chunks, the "output.dir" option must be used, not "output.file".' +
(typeof inputOptions.input !== 'string' ||
inputOptions.inlineDynamicImports === true
? ''
: ' To inline dynamic imports, set the "inlineDynamicImports" option.')
});
}) as any
};
if (inputOptions.perf === true) result.getTimings = getTimings;
return result;
}
return Promise.all(
Object.keys(bundle).map(chunkId =>
writeOutputFile(graph, result, bundle[chunkId], outputOptions)
)
)
.then(() => graph.pluginDriver.hookParallel('writeBundle', [bundle]))
.then(() => createOutput(bundle));
});
} catch (err) {
return Promise.reject(err);
}
}) as any
};
if (inputOptions.perf === true) result.getTimings = getTimings;
return result;
}

enum SortingFileType {
Expand Down