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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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'
}
}
};
Original file line number Diff line number Diff line change
@@ -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 });

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



return index.myBasePkg;

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

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

return module;

});
Original file line number Diff line number Diff line change
@@ -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 });

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

var underBuild = {
base: index.myBasePkg
};

return underBuild;

});
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

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



module.exports = index.myBasePkg;
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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 };
Original file line number Diff line number Diff line change
@@ -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';
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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 };
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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');
}

}
};
});
Original file line number Diff line number Diff line change
@@ -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 () {



}
};
});
Original file line number Diff line number Diff line change
@@ -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,
});

}
};
});