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

Bug/issue 820 deeply relative ESM paths in node modules not working #832

Merged
1 change: 1 addition & 0 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"@lion/calendar": "^0.16.7",
"@mapbox/rehype-prism": "^0.5.0",
"@material/mwc-button": "^0.25.2",
"@stencil/core": "^2.12.0",
"@types/trusted-types": "^2.0.2",
"lit": "^2.0.0",
"lit-redux-router": "~0.20.0",
Expand Down
80 changes: 45 additions & 35 deletions packages/cli/src/plugins/resource/plugin-node-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,19 @@ const updateImportMap = (entry, entryPath) => {
entryPath = `${entryPath}.js`;
}

importMap[entry] = entryPath;
// handle WIn v Unix-style path separators and force to /
importMap[entry.replace(/\\/g, '/')] = entryPath.replace(/\\/g, '/');
};

// handle ESM paths that have varying levels of nesting, e.g. export * from '../../something.js'
// https://github.com/ProjectEvergreen/greenwood/issues/820
async function resolveRelativeSpecifier(specifier, modulePath, dependency) {
const absoluteNodeModulesLocation = await getNodeModulesLocationForPackage(dependency);

// handle WIn v Unix-style path separators and force to /
return `${dependency}${path.join(path.dirname(modulePath), specifier).replace(/\\/g, '/').replace(absoluteNodeModulesLocation.replace(/\\/g, '/', ''), '')}`;
}

const getPackageEntryPath = async (packageJson) => {
let entry = packageJson.exports
? Object.keys(packageJson.exports) // first favor export maps first
Expand All @@ -41,58 +51,64 @@ const getPackageEntryPath = async (packageJson) => {
return entry;
};

const walkModule = async (module, dependency) => {
walk.simple(acorn.parse(module, {
const walkModule = async (modulePath, dependency) => {
const moduleContents = fs.readFileSync(modulePath, 'utf-8');

walk.simple(acorn.parse(moduleContents, {
ecmaVersion: '2020',
sourceType: 'module'
}), {
async ImportDeclaration(node) {
let { value: sourceValue } = node.source;
const absoluteNodeModulesLocation = await getNodeModulesLocationForPackage(dependency);
const isBarePath = sourceValue.indexOf('http') !== 0 && sourceValue.charAt(0) !== '.' && sourceValue.charAt(0) !== path.sep;
const hasExtension = path.extname(sourceValue) !== '';

if (path.extname(sourceValue) === '' && sourceValue.indexOf('http') !== 0 && sourceValue.indexOf('./') < 0) {
if (isBarePath && !hasExtension) {
if (!importMap[sourceValue]) {
// found a _new_ bare import for ${sourceValue}
// we should add this to the importMap and walk its package.json for more transitive deps
updateImportMap(sourceValue, `/node_modules/${sourceValue}`);
}

await walkPackageJson(path.join(absoluteNodeModulesLocation, 'package.json'));
} else if (sourceValue.indexOf('./') < 0) {
// adding a relative import
} else if (isBarePath) {
updateImportMap(sourceValue, `/node_modules/${sourceValue}`);
} else {
// walk this module for all its dependencies
sourceValue = sourceValue.indexOf('.js') < 0
sourceValue = !hasExtension
? `${sourceValue}.js`
: sourceValue;

if (fs.existsSync(path.join(absoluteNodeModulesLocation, sourceValue))) {
const moduleContents = fs.readFileSync(path.join(absoluteNodeModulesLocation, sourceValue));
await walkModule(moduleContents, dependency);
updateImportMap(`${dependency}/${sourceValue.replace('./', '')}`, `/node_modules/${dependency}/${sourceValue.replace('./', '')}`);
const entry = `/node_modules/${await resolveRelativeSpecifier(sourceValue, modulePath, dependency)}`;
await walkModule(path.join(absoluteNodeModulesLocation, sourceValue), dependency);

updateImportMap(path.join(dependency, sourceValue), entry);
}
}

return Promise.resolve();
},
ExportNamedDeclaration(node) {
async ExportNamedDeclaration(node) {
const sourceValue = node && node.source ? node.source.value : '';

if (sourceValue !== '' && sourceValue.indexOf('http') !== 0) {
// handle relative specifier
if (sourceValue.indexOf('.') === 0) {
updateImportMap(`${dependency}/${sourceValue.replace('./', '')}`, `/node_modules/${dependency}/${sourceValue.replace('./', '')}`);
const entry = `/node_modules/${await resolveRelativeSpecifier(sourceValue, modulePath, dependency)}`;

updateImportMap(path.join(dependency, sourceValue), entry);
} else {
// handle bare specifier
updateImportMap(sourceValue, `/node_modules/${sourceValue}`);
}
}
},
ExportAllDeclaration(node) {
async ExportAllDeclaration(node) {
const sourceValue = node && node.source ? node.source.value : '';

if (sourceValue !== '' && sourceValue.indexOf('http') !== 0) {
if (sourceValue.indexOf('.') === 0) {
updateImportMap(`${dependency}/${sourceValue.replace('./', '')}`, `/node_modules/${dependency}/${sourceValue.replace('./', '')}`);
const entry = `/node_modules/${await resolveRelativeSpecifier(sourceValue, modulePath, dependency)}`;

updateImportMap(path.join(dependency, sourceValue), entry);
} else {
updateImportMap(sourceValue, `/node_modules/${sourceValue}`);
}
Expand All @@ -107,7 +123,7 @@ const walkPackageJson = async (packageJson = {}) => {
// and walk its package.json for its dependencies

for (const dependency of Object.keys(packageJson.dependencies || {})) {
const dependencyPackageRootPath = path.join(process.cwd(), './node_modules', dependency);
const dependencyPackageRootPath = path.join(process.cwd(), 'node_modules', dependency);
const dependencyPackageJsonPath = path.join(dependencyPackageRootPath, 'package.json');
const dependencyPackageJson = JSON.parse(fs.readFileSync(dependencyPackageJsonPath, 'utf-8'));
const entry = await getPackageEntryPath(dependencyPackageJson);
Expand Down Expand Up @@ -137,7 +153,7 @@ const walkPackageJson = async (packageJson = {}) => {
break;
case 'object':
const entryTypes = Object.keys(mapItem);

if (entryTypes.import) {
esmPath = entryTypes.import;
} else if (entryTypes.require) {
Expand Down Expand Up @@ -165,31 +181,26 @@ const walkPackageJson = async (packageJson = {}) => {

// use the dependency itself as an entry in the importMap
if (entry === '.') {
updateImportMap(dependency, `/node_modules/${dependency}/${packageExport.replace('./', '')}`);
updateImportMap(dependency, `/node_modules/${path.join(dependency, packageExport)}`);
}
} else if (exportMapEntry.endsWith && (exportMapEntry.endsWith('.js') || exportMapEntry.endsWith('.mjs')) && exportMapEntry.indexOf('*') < 0) {
// is probably a file, so _not_ an export array, package.json, or wildcard export
packageExport = exportMapEntry;
}

if (packageExport) {
const packageExportLocation = path.join(absoluteNodeModulesLocation, packageExport.replace('./', ''));
const packageExportLocation = path.resolve(absoluteNodeModulesLocation, packageExport);

// check all exports of an exportMap entry
// to make sure those deps get added to the importMap
if (packageExport.endsWith('js')) {
const moduleContents = fs.readFileSync(packageExportLocation);

await walkModule(moduleContents, dependency);
updateImportMap(`${dependency}${entry.replace('.', '')}`, `/node_modules/${dependency}/${packageExport.replace('./', '')}`);
updateImportMap(path.join(dependency, entry), `/node_modules/${path.join(dependency, packageExport)}`);
} else if (fs.lstatSync(packageExportLocation).isDirectory()) {
fs.readdirSync(packageExportLocation)
.filter(file => file.endsWith('.js') || file.endsWith('.mjs'))
.forEach((file) => {
updateImportMap(`${dependency}/${packageExport.replace('./', '')}${file}`, `/node_modules/${dependency}/${packageExport.replace('./', '')}${file}`);
updateImportMap(path.join(dependency, packageExport, file), `/node_modules/${path.join(dependency, packageExport, file)}`);
});
} else {
console.warn('Warning, not able to handle export', `${dependency}/${packageExport}`);
console.warn('Warning, not able to handle export', path.join(dependency, packageExport));
}
}
}
Expand All @@ -200,10 +211,9 @@ const walkPackageJson = async (packageJson = {}) => {

// sometimes a main file is actually just an empty string... :/
if (fs.existsSync(packageEntryPointPath)) {
const packageEntryModule = fs.readFileSync(packageEntryPointPath, 'utf-8');

await walkModule(packageEntryModule, dependency);
updateImportMap(dependency, `/node_modules/${dependency}/${entry}`);
updateImportMap(dependency, `/node_modules/${path.join(dependency, entry)}`);

await walkModule(packageEntryPointPath, dependency);
await walkPackageJson(dependencyPackageJson);
}
}
Expand Down
17 changes: 16 additions & 1 deletion packages/cli/test/cases/develop.default/develop.default.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,18 @@ describe('Develop Greenwood With: ', function() {
`${process.cwd()}/node_modules/tslib/*.js`,
`${outputPath}/node_modules/tslib/`
);
const stencilCorePackageJson = await getDependencyFiles(
`${process.cwd()}/node_modules/@stencil/core/package.json`,
`${outputPath}/node_modules/@stencil/core/`
);
const stencilCoreCoreLibs = await getDependencyFiles(
`${process.cwd()}/node_modules/@stencil/core/internal/stencil-core/*.js`,
`${outputPath}/node_modules/@stencil/core/internal/stencil-core/`
);
const stencilCoreClientLibs = await getDependencyFiles(
`${process.cwd()}/node_modules/@stencil/core/internal/client/*.js`,
`${outputPath}/node_modules/@stencil/core/internal/client/`
);

// manually copy all these @babel/runtime files recursively since there are too many of them to do it individually
const babelRuntimeLibs = await rreaddir(`${process.cwd()}/node_modules/@babel/runtime`);
Expand Down Expand Up @@ -437,7 +449,10 @@ describe('Develop Greenwood With: ', function() {
...materialThemePackageJson,
...materialThemeLibs,
...tslibPackageJson,
...tslibLibs
...tslibLibs,
...stencilCorePackageJson,
...stencilCoreCoreLibs,
...stencilCoreClientLibs
]);

return new Promise(async (resolve) => {
Expand Down
10 changes: 3 additions & 7 deletions packages/cli/test/cases/develop.default/import-map.snapshot.json
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,8 @@
"@lion/button": "/node_modules/@lion/button/index.js",
"@lion/core": "/node_modules/@lion/core/index.js",
"@lion/core/differentKeyEventNamesShimIE": "/node_modules/@lion/core/src/differentKeyEventNamesShimIE.js",
"@lion/button/src/LionButton.js": "/node_modules/@lion/button/src/LionButton.js",
"@lion/button/define-button": "/node_modules/@lion/button/lion-button.js",
"@lion/button/src/LionButtonReset.js": "/node_modules/@lion/button/src/LionButtonReset.js",
"@lion/button/define-button-reset": "/node_modules/@lion/button/lion-button-reset.js",
"@lion/button/src/LionButtonSubmit.js": "/node_modules/@lion/button/src/LionButtonSubmit.js",
"@lion/button/define-button-submit": "/node_modules/@lion/button/lion-button-submit.js",
"@lion/button/define": "/node_modules/@lion/button/define.js",
"lit": "/node_modules/lit/index.js",
Expand Down Expand Up @@ -258,17 +255,16 @@
"lit-element/decorators/state.js": "/node_modules/lit-element/decorators/state.js",
"lit-element/polyfill-support.js": "/node_modules/lit-element/polyfill-support.js",
"lit-element/private-ssr-support.js": "/node_modules/lit-element/private-ssr-support.js",
"lit-html/lit-html.js": "/node_modules/lit-html/lit-html.js",
"lit-html/polyfill-support.js": "/node_modules/lit-html/polyfill-support.js",
"lit-html/private-ssr-support.js": "/node_modules/lit-html/private-ssr-support.js",
"@lion/calendar": "/node_modules/@lion/calendar/index.js",
"@lion/localize": "/node_modules/@lion/localize/index.js",
"@lion/calendar/src/LionCalendar.js": "/node_modules/@lion/calendar/src/LionCalendar.js",
"@lion/calendar/define": "/node_modules/@lion/calendar/lion-calendar.js",
"@lion/calendar/test-helpers": "/node_modules/@lion/calendar/test-helpers/index.js",
"@lion/localize/test-helpers": "/node_modules/@lion/localize/test-helpers/index.js",
"@bundled-es-modules/message-format/MessageFormat.js": "/node_modules/@bundled-es-modules/message-format/MessageFormat.js",
"@bundled-es-modules/message-format": "/node_modules/@bundled-es-modules/message-format/index.js",
"singleton-manager/src/SingletonManagerClass.js": "/node_modules/singleton-manager/src/SingletonManagerClass.js",
"singleton-manager": "/node_modules/singleton-manager/index.js"
"singleton-manager": "/node_modules/singleton-manager/index.js",
"@stencil/core": "/node_modules/@stencil/core/internal/stencil-core/index.js",
"@stencil/client/index.js": "/node_modules/@stencil/core/internal/client/index.js"
}
1 change: 1 addition & 0 deletions packages/cli/test/cases/develop.default/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"@lion/button": "^0.14.3",
"@lion/calendar": "^0.16.5",
"@material/mwc-button": "^0.25.2",
"@stencil/core": "^2.12.0",
"@types/trusted-types": "^2.0.2",
"lit": "^2.0.0"
}
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2481,6 +2481,11 @@
resolved "https://registry.yarnpkg.com/@rollup/stream/-/stream-2.0.0.tgz#2ada818c2d042e37f63119d7bf8bbfc71792f641"
integrity sha512-HsCyY/phZMys1zFUYoYlnDJGG9zMmYFfEjDKNQa00CYgjeyGD4cLdO6KNIkBh61AWOZfOsTPuGtNmFCsjQOfFg==

"@stencil/core@^2.12.0":
version "2.12.0"
resolved "https://registry.yarnpkg.com/@stencil/core/-/core-2.12.0.tgz#5b12517dd367908026692d3b00fa1aab39638ab9"
integrity sha512-hQlQKh5CUJe8g3L5avLLsfgVu95HMS2LToTtS7gpvvP0eKes1VvAC56uhI+vH4u44GZl9ck/g1rJBVRmMWu0LA==

"@stylelint/postcss-css-in-js@^0.37.2":
version "0.37.2"
resolved "https://registry.yarnpkg.com/@stylelint/postcss-css-in-js/-/postcss-css-in-js-0.37.2.tgz#7e5a84ad181f4234a2480803422a47b8749af3d2"
Expand Down