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 17 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 prepended 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 prepended 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,15 +1053,15 @@ 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:

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

An ID to use for AMD/UMD bundles:
An ID to use for AMD/UMD bundles.

```js
// rollup.config.js
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
19 changes: 11 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,18 @@ export default class Chunk {
});
}

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 +757,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
15 changes: 15 additions & 0 deletions src/finalisers/shared/getCompleteAmdId.ts
@@ -0,0 +1,15 @@
import { NormalizedOutputOptions } from '../../rollup/types';
import removeJsExtension from './removeJsExtension';

export default function getCompleteAmdId(
{ id, autoId, basePath }: NormalizedOutputOptions['amd'],
chunkId: string
): string {
if (id) {
return id;
} else if (autoId) {
return `${basePath ? basePath + '/' : ''}${removeJsExtension(chunkId)}`;
} else {
return '';
}
}
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
16 changes: 12 additions & 4 deletions src/rollup/types.d.ts
Expand Up @@ -320,8 +320,12 @@ export type ResolveFileUrlHook = (
export type AddonHookFunction = (this: PluginContext) => string | Promise<string>;
export type AddonHook = string | AddonHookFunction;

export type ChangeEvent = 'create' | 'update' | 'delete'
export type WatchChangeHook = (this: PluginContext, id: string, change: {event: ChangeEvent}) => void
export type ChangeEvent = 'create' | 'update' | 'delete';
export type WatchChangeHook = (
this: PluginContext,
id: string,
change: { event: ChangeEvent }
) => void;

/**
* use this type for plugin annotation
Expand Down Expand Up @@ -574,6 +578,8 @@ export type GetInterop = (id: string | null) => InteropType;

export interface OutputOptions {
amd?: {
autoId?: boolean;
basePath?: string;
tjenkinson marked this conversation as resolved.
Show resolved Hide resolved
define?: string;
id?: string;
};
Expand Down Expand Up @@ -622,6 +628,8 @@ export interface OutputOptions {

export interface NormalizedOutputOptions {
amd: {
autoId: boolean;
basePath: string;
define: string;
id?: string;
};
Expand Down Expand Up @@ -794,7 +802,7 @@ export interface RollupWatchOptions extends InputOptions {
watch?: WatcherOptions | false;
}

interface TypedEventEmitter<T extends {[event: string]: (...args: any) => any}> {
interface TypedEventEmitter<T extends { [event: string]: (...args: any) => any }> {
addListener<K extends keyof T>(event: K, listener: T[K]): this;
emit<K extends keyof T>(event: K, ...args: Parameters<T[K]>): boolean;
eventNames(): Array<keyof T>;
Expand Down Expand Up @@ -827,7 +835,7 @@ export type RollupWatcherEvent =

export interface RollupWatcher
extends TypedEventEmitter<{
change: (id: string, change: {event: ChangeEvent}) => void;
change: (id: string, change: { event: ChangeEvent }) => void;
close: () => void;
event: (event: RollupWatcherEvent) => void;
restart: () => void;
Expand Down
7 changes: 4 additions & 3 deletions src/utils/FileEmitter.ts
Expand Up @@ -41,7 +41,9 @@ function generateAssetFileName(
const emittedName = name || 'asset';
return makeUnique(
renderNamePattern(
output.assetFileNames,
typeof output.assetFileNames === 'function'
? output.assetFileNames({ name, source, type: 'asset' })
: output.assetFileNames,
'output.assetFileNames',
{
hash() {
Expand All @@ -54,8 +56,7 @@ function generateAssetFileName(
ext: () => extname(emittedName).substr(1),
extname: () => extname(emittedName),
name: () => emittedName.substr(0, emittedName.length - extname(emittedName).length)
},
() => ({ name, source, type: 'asset' })
}
),
output.bundle
);
Expand Down
34 changes: 22 additions & 12 deletions src/utils/options/normalizeOutputOptions.ts
Expand Up @@ -179,18 +179,28 @@ const getPreserveModulesRoot = (config: GenericConfigObject): string | undefined
return resolve(preserveModulesRoot);
};

const getAmd = (
config: GenericConfigObject
): {
define: string;
id?: string;
} => ({
define: 'define',
...(config.amd as {
define?: string;
id?: string;
})
});
const getAmd = (config: GenericConfigObject): NormalizedOutputOptions['amd'] => {
const normalized = {
autoId: false,
basePath: '',
define: 'define',
...(config.amd as OutputOptions['amd'])
};
if ((normalized.autoId || normalized.basePath) && normalized.id) {
return error({
code: 'INVALID_OPTION',
message:
'"output.amd.autoId"/"output.amd.basePath" and "output.amd.id" cannot be used together.'
});
}
if (normalized.basePath && !normalized.autoId) {
return error({
code: 'INVALID_OPTION',
message: '"output.amd.basePath" only works with "output.amd.autoId".'
});
}
return normalized;
};

const getAddon = (config: GenericConfigObject, name: string): (() => string | Promise<string>) => {
const configAddon = config[name] as string | (() => string | Promise<string>);
Expand Down