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

[Feature] Add preserveModulesRoot config option #3786

Merged
merged 11 commits into from Sep 21, 2020
1 change: 1 addition & 0 deletions cli/help.md
Expand Up @@ -46,6 +46,7 @@ Basic options:
--preferConst Use `const` instead of `var` for exports
--no-preserveEntrySignatures Avoid facade chunks for entry points
--preserveModules Preserve module structure
--preserveModulesRoot Preserved modules under this path are rooted in output `dir`
--preserveSymlinks Do not follow symlinks when resolving files
--shimMissingExports Create shim variables for missing exports
--silent Don't print warnings
Expand Down
2 changes: 2 additions & 0 deletions docs/01-command-line-reference.md
Expand Up @@ -78,6 +78,7 @@ export default { // can be an array (for multiple inputs)
outro,
paths,
preserveModules,
preserveModulesRoot,
sourcemap,
sourcemapExcludeSources,
sourcemapFile,
Expand Down Expand Up @@ -303,6 +304,7 @@ Many options have command line equivalents. In those cases, any arguments passed
--preferConst Use `const` instead of `var` for exports
--no-preserveEntrySignatures Avoid facade chunks for entry points
--preserveModules Preserve module structure
--preserveModulesRoot Preserved modules under this path are rooted in output `dir`
--preserveSymlinks Do not follow symlinks when resolving files
--shimMissingExports Create shim variables for missing exports
--silent Don't print warnings
Expand Down
3 changes: 2 additions & 1 deletion docs/02-javascript-api.md
Expand Up @@ -138,6 +138,7 @@ const outputOptions = {
outro,
paths,
preserveModules,
preserveModulesRoot,
sourcemap,
sourcemapExcludeSources,
sourcemapFile,
Expand Down Expand Up @@ -235,4 +236,4 @@ loadConfigFile(path.resolve(__dirname, 'rollup.config.js'), { format: 'es' }).th
rollup.watch(options);
}
);
```
```
35 changes: 22 additions & 13 deletions src/Chunk.ts
Expand Up @@ -441,20 +441,29 @@ export default class Chunk {
? '[name].js'
: '[name][extname].js'
: options.entryFileNames;
path = relative(
preserveModulesRelativeDir,
`${dirname(sanitizedId)}/${renderNamePattern(
pattern,
'output.entryFileNames',
{
ext: () => extension.substr(1),
extname: () => extension,
format: () => options.format as string,
name: () => this.getChunkName()
},
this.getChunkInfo.bind(this)
)}`
let currentDir = dirname(sanitizedId);
const fileName = renderNamePattern(
pattern,
'output.entryFileNames',
{
ext: () => extension.substr(1),
extname: () => extension,
format: () => options.format as string,
name: () => this.getChunkName()
},
this.getChunkInfo.bind(this)
);
if (options.preserveModulesRoot) {
const preserveModulesRoot = resolve(options.preserveModulesRoot);
Copy link
Member

Choose a reason for hiding this comment

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

There is quite a bit of resolving going on here which is not free as resolve involves non-trivial file-system operations. So one way to get rid of the first one at least would be to perform this resolution once in normalizeOutputOptions so that options.preserveModulesRoot is already resolved at this point.

if (currentDir.startsWith(preserveModulesRoot)) {
currentDir = resolve(
preserveModulesRelativeDir,
currentDir.slice(preserveModulesRoot.length).replace(/^\//, '')
);
}
}
const currentPath = `${currentDir}/${fileName}`;
path = relative(preserveModulesRelativeDir, currentPath);
Copy link
Member

Choose a reason for hiding this comment

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

I am thinking, is this really all necessary? Wouldn't this be the same:

if (currentDir.startsWith(preserveModulesRoot)) {
  path = currentDir.slice(preserveModulesRoot.length).replace(/^\//, '')
} else {
  path = relative(preserveModulesRelativeDir, `${currentDir}/${fileName}`);
}

or am I overlooking something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In practice, this will probably will be the case, at least for right now. We would need to add a check somewhere to make sure preserveModulesRelativeDir is resolved as a parent directory to preserveModulesRoot for this to work out.

Edit: I think I figured this out -- this was close to what we needed

} else {
path = `_virtual/${basename(sanitizedId)}`;
}
Expand Down
2 changes: 2 additions & 0 deletions src/rollup/types.d.ts
Expand Up @@ -583,6 +583,7 @@ export interface OutputOptions {
plugins?: OutputPlugin[];
preferConst?: boolean;
preserveModules?: boolean;
preserveModulesRoot?: string;
sourcemap?: boolean | 'inline' | 'hidden';
sourcemapExcludeSources?: boolean;
sourcemapFile?: string;
Expand Down Expand Up @@ -628,6 +629,7 @@ export interface NormalizedOutputOptions {
plugins: OutputPlugin[];
preferConst: boolean;
preserveModules: boolean;
preserveModulesRoot: string | undefined;
sourcemap: boolean | 'inline' | 'hidden';
sourcemapExcludeSources: boolean;
sourcemapFile: string | undefined;
Expand Down
1 change: 1 addition & 0 deletions src/utils/options/mergeOptions.ts
Expand Up @@ -221,6 +221,7 @@ function mergeOutputOptions(
plugins: ensureArray(config.plugins) as Plugin[],
preferConst: getOption('preferConst'),
preserveModules: getOption('preserveModules'),
preserveModulesRoot: getOption('preserveModulesRoot'),
sourcemap: getOption('sourcemap'),
sourcemapExcludeSources: getOption('sourcemapExcludeSources'),
sourcemapFile: getOption('sourcemapFile'),
Expand Down
1 change: 1 addition & 0 deletions src/utils/options/normalizeOutputOptions.ts
Expand Up @@ -64,6 +64,7 @@ export function normalizeOutputOptions(
plugins: ensureArray(config.plugins) as Plugin[],
preferConst: (config.preferConst as boolean | undefined) || false,
preserveModules,
preserveModulesRoot: config.preserveModulesRoot as string | undefined,
sourcemap: (config.sourcemap as boolean | 'inline' | 'hidden' | undefined) || false,
sourcemapExcludeSources: (config.sourcemapExcludeSources as boolean | undefined) || false,
sourcemapFile: config.sourcemapFile as string | undefined,
Expand Down
21 changes: 21 additions & 0 deletions test/chunking-form/samples/preserve-modules-root/_config.js
@@ -0,0 +1,21 @@
const commonjs = require('@rollup/plugin-commonjs');
const resolve = require('@rollup/plugin-node-resolve').default;

module.exports = {
description: 'confirm preserveModulesRoot restructures src appropriately',
options: {
input: ['src/under-build.js', 'src/below/module.js'],
plugins: [
resolve({
customResolveOptions: {
moduleDirectory: ['custom_modules']
}
}),
commonjs()
],
output: {
preserveModules: true,
preserveModulesRoot: 'src'
}
}
};
@@ -0,0 +1,22 @@
define(['exports'], function (exports) { 'use strict';

function createCommonjsModule(fn, basedir, module) {
return module = {
path: basedir,
exports: {},
require: function (path, base) {
return commonjsRequire(path, (base === undefined || base === null) ? module.path : base);
}
}, fn(module, module.exports), module.exports;
}

function commonjsRequire () {
throw new Error('Dynamic requires are not currently supported by @rollup/plugin-commonjs');
}

exports.commonjsRequire = commonjsRequire;
exports.createCommonjsModule = createCommonjsModule;

Object.defineProperty(exports, '__esModule', { value: true });

});
@@ -0,0 +1,7 @@
define(['../custom_modules/@my-scope/my-base-pkg/index'], function (index) { 'use strict';



return index.myBasePkg;

});
@@ -0,0 +1,9 @@
define(['../custom_modules/@my-scope/my-base-pkg/index'], function (index) { 'use strict';

var module = {
base2: index.myBasePkg,
};

return module;

});
@@ -0,0 +1,16 @@
define(['exports', '../../../_virtual/_commonjsHelpers'], function (exports, _commonjsHelpers) { 'use strict';

var myBasePkg = _commonjsHelpers.createCommonjsModule(function (module, exports) {

Object.defineProperty(exports, '__esModule', { value: true });

var hello = 'world';

exports.hello = hello;
});

exports.myBasePkg = myBasePkg;

Object.defineProperty(exports, '__esModule', { value: true });

});
@@ -0,0 +1,9 @@
define(['./custom_modules/@my-scope/my-base-pkg/index'], function (index) { 'use strict';

var underBuild = {
base: index.myBasePkg
};

return underBuild;

});
@@ -0,0 +1,20 @@
'use strict';

Object.defineProperty(exports, '__esModule', { value: true });

function createCommonjsModule(fn, basedir, module) {
return module = {
path: basedir,
exports: {},
require: function (path, base) {
return commonjsRequire(path, (base === undefined || base === null) ? module.path : base);
}
}, fn(module, module.exports), module.exports;
}

function commonjsRequire () {
throw new Error('Dynamic requires are not currently supported by @rollup/plugin-commonjs');
}

exports.commonjsRequire = commonjsRequire;
exports.createCommonjsModule = createCommonjsModule;
@@ -0,0 +1,7 @@
'use strict';

var index = require('../custom_modules/@my-scope/my-base-pkg/index.js');



module.exports = index.myBasePkg;
@@ -0,0 +1,9 @@
'use strict';

var index = require('../custom_modules/@my-scope/my-base-pkg/index.js');

var module$1 = {
base2: index.myBasePkg,
};

module.exports = module$1;
@@ -0,0 +1,16 @@
'use strict';

Object.defineProperty(exports, '__esModule', { value: true });

var _commonjsHelpers = require('../../../_virtual/_commonjsHelpers.js');

var myBasePkg = _commonjsHelpers.createCommonjsModule(function (module, exports) {

Object.defineProperty(exports, '__esModule', { value: true });

var hello = 'world';

exports.hello = hello;
});

exports.myBasePkg = myBasePkg;
@@ -0,0 +1,9 @@
'use strict';

var index = require('./custom_modules/@my-scope/my-base-pkg/index.js');

var underBuild = {
base: index.myBasePkg
};

module.exports = underBuild;
@@ -0,0 +1,15 @@
function createCommonjsModule(fn, basedir, module) {
return module = {
path: basedir,
exports: {},
require: function (path, base) {
return commonjsRequire(path, (base === undefined || base === null) ? module.path : base);
}
}, fn(module, module.exports), module.exports;
}

function commonjsRequire () {
throw new Error('Dynamic requires are not currently supported by @rollup/plugin-commonjs');
}

export { commonjsRequire, createCommonjsModule };
@@ -0,0 +1,2 @@
import { m as myBasePkg } from '../custom_modules/@my-scope/my-base-pkg/index.js';
export { m as default } from '../custom_modules/@my-scope/my-base-pkg/index.js';
@@ -0,0 +1,7 @@
import { m as myBasePkg } from '../custom_modules/@my-scope/my-base-pkg/index.js';

var module = {
base2: myBasePkg,
};

export default module;
@@ -0,0 +1,12 @@
import { createCommonjsModule } from '../../../_virtual/_commonjsHelpers.js';

var myBasePkg = createCommonjsModule(function (module, exports) {

Object.defineProperty(exports, '__esModule', { value: true });

var hello = 'world';

exports.hello = hello;
});

export { myBasePkg as m };
@@ -0,0 +1,7 @@
import { m as myBasePkg } from './custom_modules/@my-scope/my-base-pkg/index.js';

var underBuild = {
base: myBasePkg
};

export default underBuild;
@@ -0,0 +1,27 @@
System.register([], function (exports) {
'use strict';
return {
execute: function () {

exports({
commonjsRequire: commonjsRequire,
createCommonjsModule: createCommonjsModule
});

function createCommonjsModule(fn, basedir, module) {
return module = {
path: basedir,
exports: {},
require: function (path, base) {
return commonjsRequire(path, (base === undefined || base === null) ? module.path : base);
}
}, fn(module, module.exports), module.exports;
}

function commonjsRequire () {
throw new Error('Dynamic requires are not currently supported by @rollup/plugin-commonjs');
}

}
};
});
@@ -0,0 +1,15 @@
System.register(['../custom_modules/@my-scope/my-base-pkg/index.js'], function (exports) {
'use strict';
var myBasePkg;
return {
setters: [function (module) {
myBasePkg = module.m;
exports('default', module.m);
}],
execute: function () {



}
};
});
@@ -0,0 +1,16 @@
System.register(['../custom_modules/@my-scope/my-base-pkg/index.js'], function (exports) {
'use strict';
var myBasePkg;
return {
setters: [function (module) {
myBasePkg = module.m;
}],
execute: function () {

var module$1 = exports('default', {
base2: myBasePkg,
});

}
};
});