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

implement amd.autoId/amd.basePath options #3867

Merged
merged 33 commits into from Nov 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
cef70d8
implement `amd.idFromChunkName` option
Nov 13, 2020
a44bef3
update output-options-hook test
Nov 13, 2020
eadea09
update NormalizedOutputOptions type
Nov 13, 2020
462c69c
simiplify removeExtension()
Nov 13, 2020
287769a
fix `idFromChunkName` type
Nov 13, 2020
65f0921
make `idFromChunkName` required in normalized options
Nov 13, 2020
3a42519
reference actual types in normalizeOutputOptions
Nov 13, 2020
04ec70f
switch to using a pattern instead
tjenkinson Nov 15, 2020
3bcd1cd
also removeExtensionFromRelativeAmdId in UMD finaliser
tjenkinson Nov 15, 2020
02b8f21
Merge branch 'remove-extension-amd-umd' into amd-id-code-split
tjenkinson Nov 15, 2020
2e9d487
support id in output.amd.id in umd finaliser
tjenkinson Nov 15, 2020
cb76695
move test to chunking-form/samples
tjenkinson Nov 15, 2020
881b932
remove test from cli section
tjenkinson Nov 15, 2020
e3cf09c
remove idFromChunkName
tjenkinson Nov 15, 2020
95d605f
revert chunking-form index change
Nov 20, 2020
96088e2
break out to amd.autoId and amd.basePath options
Nov 20, 2020
bde40bc
Merge remote-tracking branch 'upstream/master' into amd-id-code-split
Nov 20, 2020
ad09886
remove unneeded changes
Nov 20, 2020
114364f
update docs
Nov 20, 2020
9434622
update test to also put chunks in folder
Nov 20, 2020
8eeece1
throw error if amd.id used with muli-file build
Nov 27, 2020
1654ded
more accurate amd options type
Nov 27, 2020
1d4efc9
update output.amd doc
Nov 27, 2020
84e8099
fix `@ts-expect-error` comments
Nov 27, 2020
96ea9c1
add tests for invalid options
Nov 27, 2020
c9ec971
add test for entryFileNames with output.preserveModules
Nov 27, 2020
48d01b1
ignore internal error in coverage
Nov 27, 2020
e1d9173
simplify type
Nov 27, 2020
e6944ae
fix output options test
Nov 27, 2020
9cecce3
fix type tests
Nov 27, 2020
6ef8520
improve types again
Nov 27, 2020
bba3d17
simply again
Nov 27, 2020
cbae7b8
make it a warning
tjenkinson Nov 28, 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
2 changes: 2 additions & 0 deletions cli/help.md
Expand Up @@ -20,6 +20,8 @@ Basic options:
-v, --version Show version number
-w, --watch Watch files in bundle and rebuild on changes
--amd.id <id> ID for AMD module (default is anonymous)
--amd.autoId Generate the AMD ID based off the chunk name
--amd.basePath <prefix> Path to prepend to auto generated AMD ID
--amd.define <name> Function to use in place of `define`
--assetFileNames <pattern> Name pattern for emitted assets
--banner <text> Code to insert at top of bundle (outside wrapper)
Expand Down
2 changes: 2 additions & 0 deletions docs/01-command-line-reference.md
Expand Up @@ -278,6 +278,8 @@ Many options have command line equivalents. In those cases, any arguments passed
-v, --version Show version number
-w, --watch Watch files in bundle and rebuild on changes
--amd.id <id> ID for AMD module (default is anonymous)
--amd.autoId Generate the AMD ID based off the chunk name
--amd.basePath <prefix> Path to prepend to auto generated AMD ID
--amd.define <name> Function to use in place of `define`
--assetFileNames <pattern> Name pattern for emitted assets
--banner <text> Code to insert at top of bundle (outside wrapper)
Expand Down
47 changes: 45 additions & 2 deletions docs/999-big-list-of-options.md
Expand Up @@ -1053,9 +1053,9 @@ Type: `((id: string) => string) | { [id: string]: string }`<br>
Same as [`context`](guide/en/#context), but per-module – can either be an object of `id: context` pairs, or an `id => context` function.

#### output.amd
Type: `{ id?: string, define?: string}`
Type: `{ id?: string, autoId?: boolean, basePath?: string, define?: string }`
tjenkinson marked this conversation as resolved.
Show resolved Hide resolved

An object that can contain the following properties:
Note `id` can only be used for single-file builds, and cannot be combined with `autoId`/`basePath`.

**output.amd.id**<br>
Type: `string`<br>
Expand All @@ -1076,6 +1076,49 @@ export default {
// -> define('my-bundle', ['dependency'], ...
```

**output.amd.autoId**<br>
Type: `boolean`<br>
CLI: `--amd.autoId`

Set the ID to the chunk ID (with the '.js' extension removed).

```js
// rollup.config.js
export default {
...,
format: 'amd',
amd: {
autoId: true
}
};

// -> define('main', ['dependency'], ...
// -> define('dynamic-chunk', ['dependency'], ...
```

**output.amd.basePath**<br>
Type: `string`<br>
CLI: `--amd.basePath`

The path that will be prepended to the auto generated ID. This is useful if the build is going to be placed inside another AMD project, and is not at the root.

Only valid with `output.amd.autoId`.

```js
// rollup.config.js
export default {
...,
format: 'amd',
amd: {
autoId: true
basePath: 'some/where'
}
};

// -> define('some/where/main', ['dependency'], ...
// -> define('some/where/dynamic-chunk', ['dependency'], ...
```

**output.amd.define**<br>
Type: `string`<br>
CLI: `--amd.define <defineFunctionName>`
Expand Down
16 changes: 13 additions & 3 deletions src/Bundle.ts
Expand Up @@ -8,7 +8,8 @@ import {
NormalizedOutputOptions,
OutputBundle,
OutputBundleWithPlaceholders,
OutputChunk
OutputChunk,
WarningHandler
} from './rollup/types';
import { Addons, createAddons } from './utils/addons';
import { getChunkAssignments } from './utils/chunkAssignment';
Expand Down Expand Up @@ -46,7 +47,7 @@ export default class Bundle {
timeStart('generate chunks', 2);
const chunks = await this.generateChunks();
if (chunks.length > 1) {
validateOptionsForMultiChunkOutput(this.outputOptions);
validateOptionsForMultiChunkOutput(this.outputOptions, this.inputOptions.onwarn);
}
const inputBase = commondir(getAbsoluteEntryModulePaths(chunks));
timeEnd('generate chunks', 2);
Expand Down Expand Up @@ -243,7 +244,10 @@ function getAbsoluteEntryModulePaths(chunks: Chunk[]): string[] {
return absoluteEntryModulePaths;
}

function validateOptionsForMultiChunkOutput(outputOptions: NormalizedOutputOptions) {
function validateOptionsForMultiChunkOutput(
outputOptions: NormalizedOutputOptions,
onWarn: WarningHandler
) {
if (outputOptions.format === 'umd' || outputOptions.format === 'iife')
return error({
code: 'INVALID_OPTION',
Expand All @@ -261,6 +265,12 @@ function validateOptionsForMultiChunkOutput(outputOptions: NormalizedOutputOptio
code: 'INVALID_OPTION',
message: '"output.sourcemapFile" is only supported for single-file builds.'
});
if (!outputOptions.amd.autoId && outputOptions.amd.id)
lukastaegert marked this conversation as resolved.
Show resolved Hide resolved
onWarn({
code: 'INVALID_OPTION',
message:
'"output.amd.id" is only properly supported for single-file builds. Use "output.amd.autoId" and "output.amd.basePath".'
});
}

function getIncludedModules(modulesById: Map<string, Module | ExternalModule>): Module[] {
Expand Down
20 changes: 12 additions & 8 deletions src/Chunk.ts
Expand Up @@ -414,7 +414,7 @@ export default class Chunk {
: [options.chunkFileNames, 'output.chunkFileNames'];
return makeUnique(
renderNamePattern(
pattern,
typeof pattern === 'function' ? pattern(this.getChunkInfo()) : pattern,
patternName,
{
format: () => options.format,
Expand All @@ -423,8 +423,7 @@ export default class Chunk {
? this.computeContentHashWithDependencies(addons, options, existingNames)
: '[hash]',
name: () => this.getChunkName()
},
this.getChunkInfo.bind(this)
}
),
existingNames
);
Expand All @@ -448,15 +447,14 @@ export default class Chunk {
: options.entryFileNames;
const currentDir = dirname(sanitizedId);
const fileName = renderNamePattern(
pattern,
typeof pattern === 'function' ? pattern(this.getChunkInfo()) : pattern,
lukastaegert marked this conversation as resolved.
Show resolved Hide resolved
'output.entryFileNames',
{
ext: () => extension.substr(1),
extname: () => extension,
format: () => options.format as string,
name: () => this.getChunkName()
},
this.getChunkInfo.bind(this)
}
);
const currentPath = `${currentDir}/${fileName}`;
const { preserveModulesRoot } = options;
Expand Down Expand Up @@ -712,13 +710,19 @@ export default class Chunk {
});
}

/* istanbul ignore next */
if (!this.id) {
Copy link
Member

Choose a reason for hiding this comment

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

Not a big fan of internal errors that are impossible to test, but there are other opinions out there as well. The price of course are non-null assertions. In a future version of Rollup, maybe we can move away from classes towards data types that evolve as the bundle evolves.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeh I had that initially but noticed a few more internal errors. Happy to remove if you want

maybe we can move away from classes towards data types that evolve as the bundle evolves.

sounds good

throw new Error('Internal Error: expecting chunk id');
}

const magicString = finalise(
this.renderedSource!,
{
accessedGlobals,
dependencies: [...this.renderedDependencies!.values()],
exports: this.renderedExports!,
hasExports,
id: this.id,
indentString: this.indentString,
intro: addons.intro!,
isEntryFacade:
Expand Down Expand Up @@ -754,8 +758,8 @@ export default class Chunk {

let file: string;
if (options.file) file = resolve(options.sourcemapFile || options.file);
else if (options.dir) file = resolve(options.dir, this.id!);
else file = resolve(this.id!);
else if (options.dir) file = resolve(options.dir, this.id);
else file = resolve(this.id);

const decodedMap = magicString.generateDecodedMap({});
map = collapseSourcemaps(
Expand Down
10 changes: 7 additions & 3 deletions src/finalisers/amd.ts
@@ -1,6 +1,7 @@
import { Bundle as MagicStringBundle } from 'magic-string';
import { NormalizedOutputOptions } from '../rollup/types';
import { FinaliserOptions } from './index';
import getCompleteAmdId from './shared/getCompleteAmdId';
import { getExportBlock, getNamespaceMarkers } from './shared/getExportBlock';
import getInteropBlock from './shared/getInteropBlock';
import removeExtensionFromRelativeAmdId from './shared/removeExtensionFromRelativeAmdId';
Expand All @@ -13,6 +14,7 @@ export default function amd(
dependencies,
exports,
hasExports,
id,
indentString: t,
intro,
isEntryFacade,
Expand All @@ -23,7 +25,7 @@ export default function amd(
warn
}: FinaliserOptions,
{
amd: { define: amdDefine, id: amdId },
amd,
compact,
esModule,
externalLiveBindings,
Expand Down Expand Up @@ -55,8 +57,10 @@ export default function amd(
deps.unshift(`'module'`);
}

const completeAmdId = getCompleteAmdId(amd, id);
const params =
(amdId ? `'${amdId}',${_}` : ``) + (deps.length ? `[${deps.join(`,${_}`)}],${_}` : ``);
(completeAmdId ? `'${completeAmdId}',${_}` : ``) +
(deps.length ? `[${deps.join(`,${_}`)}],${_}` : ``);
const useStrict = strict ? `${_}'use strict';` : '';

magicString.prepend(
Expand Down Expand Up @@ -97,6 +101,6 @@ export default function amd(
magicString.append(`${exportBlock}${namespaceMarkers}${outro}`);
return magicString
.indent(t)
.prepend(`${amdDefine}(${params}function${_}(${args.join(`,${_}`)})${_}{${useStrict}${n}${n}`)
.prepend(`${amd.define}(${params}function${_}(${args.join(`,${_}`)})${_}{${useStrict}${n}${n}`)
.append(`${n}${n}});`);
}
1 change: 1 addition & 0 deletions src/finalisers/index.ts
Expand Up @@ -13,6 +13,7 @@ export interface FinaliserOptions {
dependencies: ChunkDependencies;
exports: ChunkExports;
hasExports: boolean;
id: string;
indentString: string;
intro: string;
isEntryFacade: boolean;
Expand Down
13 changes: 13 additions & 0 deletions src/finalisers/shared/getCompleteAmdId.ts
@@ -0,0 +1,13 @@
import { NormalizedOutputOptions } from '../../rollup/types';
import removeJsExtension from './removeJsExtension';

export default function getCompleteAmdId(
options: NormalizedOutputOptions['amd'],
chunkId: string
): string {
if (!options.autoId) {
return options.id || '';
} else {
return `${options.basePath ? options.basePath + '/' : ''}${removeJsExtension(chunkId)}`;
}
}
7 changes: 3 additions & 4 deletions src/finalisers/shared/removeExtensionFromRelativeAmdId.ts
@@ -1,9 +1,8 @@
import removeJsExtension from './removeJsExtension';

// AMD resolution will only respect the AMD baseUrl if the .js extension is omitted.
// The assumption is that this makes sense for all relative ids:
// https://requirejs.org/docs/api.html#jsfiles
export default function removeExtensionFromRelativeAmdId(id: string) {
if (id[0] === '.' && id.endsWith('.js')) {
return id.slice(0, -3);
}
return id;
return id[0] === '.' ? removeJsExtension(id) : id;
}
3 changes: 3 additions & 0 deletions src/finalisers/shared/removeJsExtension.ts
@@ -0,0 +1,3 @@
export default function removeJsExtension(name: string) {
return name.endsWith('.js') ? name.slice(0, -3) : name;
}
10 changes: 7 additions & 3 deletions src/finalisers/umd.ts
Expand Up @@ -2,6 +2,7 @@ import { Bundle as MagicStringBundle } from 'magic-string';
import { NormalizedOutputOptions } from '../rollup/types';
import { error } from '../utils/error';
import { FinaliserOptions } from './index';
import getCompleteAmdId from './shared/getCompleteAmdId';
import { getExportBlock, getNamespaceMarkers } from './shared/getExportBlock';
import getInteropBlock from './shared/getInteropBlock';
import removeExtensionFromRelativeAmdId from './shared/removeExtensionFromRelativeAmdId';
Expand Down Expand Up @@ -29,6 +30,7 @@ export default function umd(
dependencies,
exports,
hasExports,
id,
indentString: t,
intro,
namedExportsMode,
Expand All @@ -37,7 +39,7 @@ export default function umd(
warn
}: FinaliserOptions,
{
amd: { define: amdDefine, id: amdId },
amd,
compact,
esModule,
extend,
Expand Down Expand Up @@ -90,10 +92,12 @@ export default function umd(
factoryArgs.unshift('exports');
}

const completeAmdId = getCompleteAmdId(amd, id);
const amdParams =
(amdId ? `'${amdId}',${_}` : ``) + (amdDeps.length ? `[${amdDeps.join(`,${_}`)}],${_}` : ``);
(completeAmdId ? `'${completeAmdId}',${_}` : ``) +
(amdDeps.length ? `[${amdDeps.join(`,${_}`)}],${_}` : ``);

const define = amdDefine;
const define = amd.define;
const cjsExport = !namedExportsMode && hasExports ? `module.exports${_}=${_}` : ``;
const useStrict = strict ? `${_}'use strict';${n}` : ``;

Expand Down