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

feat: add option to keep extensions for amd #4607

Merged
merged 9 commits into from Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions cli/help.md
Expand Up @@ -23,6 +23,7 @@ Basic options:
--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`
--amd.forceJsExtensionForImports Use `.js` extension in AMD imports
--assetFileNames <pattern> Name pattern for emitted assets
--banner <text> Code to insert at top of bundle (outside wrapper)
--chunkFileNames <pattern> Name pattern for emitted secondary chunks
Expand Down
1 change: 1 addition & 0 deletions docs/01-command-line-reference.md
Expand Up @@ -358,6 +358,7 @@ Many options have command line equivalents. In those cases, any arguments passed
--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`
--amd.forceJsExtensionForImports Use `.js` extension in AMD imports
--assetFileNames <pattern> Name pattern for emitted assets
--banner <text> Code to insert at top of bundle (outside wrapper)
--chunkFileNames <pattern> Name pattern for emitted secondary chunks
Expand Down
17 changes: 17 additions & 0 deletions docs/999-big-list-of-options.md
Expand Up @@ -1331,6 +1331,23 @@ export default {
// -> def(['dependency'],...
```

**output.amd.forceJsExtensionForImports**<br> Type: `boolean`<br> CLI: `--amd.forceJsExtensionForImports`<br> Default: `false`

Add `.js` extension for imports of generated chunks and local AMD modules:

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

// -> define(['./chunk-or-local-file.js', 'dependency', 'third/dependency'],...
```

#### output.esModule

Type: `boolean`<br> CLI: `--esModule`/`--no-esModule`<br> Default: `true`
Expand Down
3 changes: 2 additions & 1 deletion src/Chunk.ts
Expand Up @@ -938,7 +938,8 @@ export default class Chunk {
options: NormalizedOutputOptions,
snippets: GenerateCodeSnippets
): void {
const stripKnownJsExtensions = options.format === 'amd';
const stripKnownJsExtensions =
options.format === 'amd' && !options.amd.forceJsExtensionForImports;
for (const [module, code] of this.renderedModuleSources) {
for (const { node, resolution } of module.dynamicImports) {
const chunk = this.chunkByModule.get(resolution as Module);
Expand Down
6 changes: 4 additions & 2 deletions src/finalisers/amd.ts
Expand Up @@ -3,7 +3,7 @@ import type { NormalizedOutputOptions } from '../rollup/types';
import getCompleteAmdId from './shared/getCompleteAmdId';
import { getExportBlock, getNamespaceMarkers } from './shared/getExportBlock';
import getInteropBlock from './shared/getInteropBlock';
import removeExtensionFromRelativeAmdId from './shared/removeExtensionFromRelativeAmdId';
import updateExtensionForRelativeAmdId from './shared/updateExtensionForRelativeAmdId';
import warnOnBuiltins from './shared/warnOnBuiltins';
import type { FinaliserOptions } from './index';

Expand Down Expand Up @@ -35,7 +35,9 @@ export default function amd(
}: NormalizedOutputOptions
): Bundle {
warnOnBuiltins(warn, dependencies);
const deps = dependencies.map(m => `'${removeExtensionFromRelativeAmdId(m.id)}'`);
const deps = dependencies.map(
m => `'${updateExtensionForRelativeAmdId(m.id, amd.forceJsExtensionForImports)}'`
);
const args = dependencies.map(m => m.name);
const { n, getNonArrowFunctionIntro, _ } = snippets;

Expand Down
3 changes: 3 additions & 0 deletions src/finalisers/shared/addJsExtension.ts
@@ -0,0 +1,3 @@
export default function addJsExtension(name: string): string {
return name.endsWith('.js') ? name : name + '.js';
}
8 changes: 0 additions & 8 deletions src/finalisers/shared/removeExtensionFromRelativeAmdId.ts

This file was deleted.

16 changes: 16 additions & 0 deletions src/finalisers/shared/updateExtensionForRelativeAmdId.ts
@@ -0,0 +1,16 @@
import addJsExtension from './addJsExtension';
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 updateExtensionForRelativeAmdId(
id: string,
forceJsExtensionForImports: boolean
): string {
if (id[0] !== '.') {
return id;
}

return forceJsExtensionForImports ? addJsExtension(id) : removeJsExtension(id);
}
6 changes: 4 additions & 2 deletions src/finalisers/umd.ts
Expand Up @@ -5,10 +5,10 @@ import type { GenerateCodeSnippets } from '../utils/generateCodeSnippets';
import getCompleteAmdId from './shared/getCompleteAmdId';
import { getExportBlock, getNamespaceMarkers } from './shared/getExportBlock';
import getInteropBlock from './shared/getInteropBlock';
import removeExtensionFromRelativeAmdId from './shared/removeExtensionFromRelativeAmdId';
import { keypath } from './shared/sanitize';
import { assignToDeepVariable } from './shared/setupNamespace';
import trimEmptyImports from './shared/trimEmptyImports';
import updateExtensionForRelativeAmdId from './shared/updateExtensionForRelativeAmdId';
import warnOnBuiltins from './shared/warnOnBuiltins';
import type { FinaliserOptions } from './index';

Expand Down Expand Up @@ -73,7 +73,9 @@ export default function umd(

warnOnBuiltins(warn, dependencies);

const amdDeps = dependencies.map(m => `'${removeExtensionFromRelativeAmdId(m.id)}'`);
const amdDeps = dependencies.map(
m => `'${updateExtensionForRelativeAmdId(m.id, amd.forceJsExtensionForImports)}'`
);
const cjsDeps = dependencies.map(m => `require('${m.id}')`);

const trimmedImports = trimEmptyImports(dependencies);
Expand Down
2 changes: 2 additions & 0 deletions src/rollup/types.d.ts
Expand Up @@ -631,6 +631,7 @@ export type AmdOptions = (
}
) & {
define?: string;
forceJsExtensionForImports?: boolean;
};

export type NormalizedAmdOptions = (
Expand All @@ -644,6 +645,7 @@ export type NormalizedAmdOptions = (
}
) & {
define: string;
forceJsExtensionForImports: boolean;
};

export interface OutputOptions {
Expand Down
13 changes: 11 additions & 2 deletions src/utils/options/normalizeOutputOptions.ts
Expand Up @@ -225,10 +225,17 @@ const getPreserveModulesRoot = (
};

const getAmd = (config: OutputOptions): NormalizedOutputOptions['amd'] => {
const mergedOption: { autoId: boolean; basePath: string; define: string; id?: string } = {
const mergedOption: {
autoId: boolean;
basePath: string;
define: string;
forceJsExtensionForImports: boolean;
id?: string;
} = {
autoId: false,
basePath: '',
define: 'define',
forceJsExtensionForImports: false,
...config.amd
};

Expand Down Expand Up @@ -256,12 +263,14 @@ const getAmd = (config: OutputOptions): NormalizedOutputOptions['amd'] => {
normalized = {
autoId: true,
basePath: mergedOption.basePath,
define: mergedOption.define
define: mergedOption.define,
forceJsExtensionForImports: mergedOption.forceJsExtensionForImports
};
} else {
normalized = {
autoId: false,
define: mergedOption.define,
forceJsExtensionForImports: mergedOption.forceJsExtensionForImports,
id: mergedOption.id
};
}
Expand Down
7 changes: 7 additions & 0 deletions test/form/samples/amd-keep-extension/_config.js
@@ -0,0 +1,7 @@
module.exports = {
description: 'keep extension for AMD modules',
options: {
external: ['./relative', 'abso/lute', './relative.js', 'abso/lute.js'],
output: { amd: { forceJsExtensionForImports: true }, interop: 'default' }
}
};
5 changes: 5 additions & 0 deletions test/form/samples/amd-keep-extension/_expected/amd.js
@@ -0,0 +1,5 @@
define(['./relative.js', 'abso/lute', './relative.js', 'abso/lute.js'], (function (relative, absolute, relativeExtension, absoluteExtension) { 'use strict';

console.log(relative, absolute, relativeExtension, absoluteExtension);

}));
8 changes: 8 additions & 0 deletions test/form/samples/amd-keep-extension/_expected/cjs.js
@@ -0,0 +1,8 @@
'use strict';

var relative = require('./relative');
var absolute = require('abso/lute');
var relativeExtension = require('./relative.js');
var absoluteExtension = require('abso/lute.js');

console.log(relative, absolute, relativeExtension, absoluteExtension);
6 changes: 6 additions & 0 deletions test/form/samples/amd-keep-extension/_expected/es.js
@@ -0,0 +1,6 @@
import relative from './relative';
import absolute from 'abso/lute';
import relativeExtension from './relative.js';
import absoluteExtension from 'abso/lute.js';

console.log(relative, absolute, relativeExtension, absoluteExtension);
6 changes: 6 additions & 0 deletions test/form/samples/amd-keep-extension/_expected/iife.js
@@ -0,0 +1,6 @@
(function (relative, absolute, relativeExtension, absoluteExtension) {
'use strict';

console.log(relative, absolute, relativeExtension, absoluteExtension);

})(relative, absolute, relativeExtension, absoluteExtension);
20 changes: 20 additions & 0 deletions test/form/samples/amd-keep-extension/_expected/system.js
@@ -0,0 +1,20 @@
System.register(['./relative', 'abso/lute', './relative.js', 'abso/lute.js'], (function () {
'use strict';
var relative, absolute, relativeExtension, absoluteExtension;
return {
setters: [function (module) {
relative = module["default"];
}, function (module) {
absolute = module["default"];
}, function (module) {
relativeExtension = module["default"];
}, function (module) {
absoluteExtension = module["default"];
}],
execute: (function () {

console.log(relative, absolute, relativeExtension, absoluteExtension);

})
};
}));
9 changes: 9 additions & 0 deletions test/form/samples/amd-keep-extension/_expected/umd.js
@@ -0,0 +1,9 @@
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(require('./relative'), require('abso/lute'), require('./relative.js'), require('abso/lute.js')) :
typeof define === 'function' && define.amd ? define(['./relative.js', 'abso/lute', './relative.js', 'abso/lute.js'], factory) :
(global = typeof globalThis !== 'undefined' ? globalThis : global || self, factory(global.relative, global.absolute, global.relativeExtension, global.absoluteExtension));
})(this, (function (relative, absolute, relativeExtension, absoluteExtension) { 'use strict';

console.log(relative, absolute, relativeExtension, absoluteExtension);

}));
6 changes: 6 additions & 0 deletions test/form/samples/amd-keep-extension/main.js
@@ -0,0 +1,6 @@
import relative from './relative';
import absolute from 'abso/lute';
import relativeExtension from './relative.js';
import absoluteExtension from 'abso/lute.js';

console.log(relative, absolute, relativeExtension, absoluteExtension);
3 changes: 2 additions & 1 deletion test/function/samples/output-options-hook/_config.js
Expand Up @@ -18,7 +18,8 @@ module.exports = {
assert.deepStrictEqual(JSON.parse(JSON.stringify(options)), {
amd: {
define: 'define',
autoId: false
autoId: false,
forceJsExtensionForImports: false
},
assetFileNames: 'assets/[name]-[hash][extname]',
chunkFileNames: '[name]-[hash].js',
Expand Down