From 840f0c272b9b4b4b4eb19bcb07838b737fa21ffe Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Mon, 20 Dec 2021 17:33:35 -0500 Subject: [PATCH 01/11] WIP supporting deep import specifier nesting --- .../plugins/resource/plugin-node-modules.js | 59 ++++++++++++++++--- 1 file changed, 50 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js index 33d4e8eae..26b20dda0 100644 --- a/packages/cli/src/plugins/resource/plugin-node-modules.js +++ b/packages/cli/src/plugins/resource/plugin-node-modules.js @@ -41,7 +41,9 @@ const getPackageEntryPath = async (packageJson) => { return entry; }; -const walkModule = async (module, dependency) => { +const walkModule = async (module, dependency, packageEntryPointPath) => { + console.debug('WALK MODULE', dependency); + console.debug('packageEntryPointPath', packageEntryPointPath); walk.simple(acorn.parse(module, { ecmaVersion: '2020', sourceType: 'module' @@ -50,14 +52,27 @@ const walkModule = async (module, dependency) => { let { value: sourceValue } = node.source; const absoluteNodeModulesLocation = await getNodeModulesLocationForPackage(dependency); + console.debug('@@@@@@@@@@ ImportDeclaration', sourceValue); + // TODO should be handled just like a ./ or ../../../ if (path.extname(sourceValue) === '' && sourceValue.indexOf('http') !== 0 && sourceValue.indexOf('./') < 0) { 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}`); + const realLocation = path.join(absoluteNodeModulesLocation.replace(dependency, ''), `${sourceValue}/index.js`); + console.debug('realLocation', realLocation); + // TODO + if (fs.existsSync(realLocation)) { + // updateImportMap(sourceValue, `/node_modules/${sourceValue}/index.js`); + // const moduleContents = fs.readFileSync(realLocation, 'utf-8'); + // console.debug('going deep!!!!'); + // await walkModule(moduleContents, dependency); + } else { + updateImportMap(sourceValue, `/node_modules/${sourceValue}`); + } } await walkPackageJson(path.join(absoluteNodeModulesLocation, 'package.json')); + // TODO should be handled just like a ./ or ../../../ } else if (sourceValue.indexOf('./') < 0) { // adding a relative import updateImportMap(sourceValue, `/node_modules/${sourceValue}`); @@ -70,31 +85,51 @@ const walkModule = async (module, dependency) => { if (fs.existsSync(path.join(absoluteNodeModulesLocation, sourceValue))) { const moduleContents = fs.readFileSync(path.join(absoluteNodeModulesLocation, sourceValue)); await walkModule(moduleContents, dependency); + // TODO should be handled just like a ./ or ../../../ updateImportMap(`${dependency}/${sourceValue.replace('./', '')}`, `/node_modules/${dependency}/${sourceValue.replace('./', '')}`); } } - - return Promise.resolve(); }, - ExportNamedDeclaration(node) { + async ExportNamedDeclaration(node) { const sourceValue = node && node.source ? node.source.value : ''; + // const absoluteNodeModulesLocation = await getNodeModulesLocationForPackage(dependency); if (sourceValue !== '' && sourceValue.indexOf('http') !== 0) { + // TODO should be handled just like a ./ or ../../../ if (sourceValue.indexOf('.') === 0) { - updateImportMap(`${dependency}/${sourceValue.replace('./', '')}`, `/node_modules/${dependency}/${sourceValue.replace('./', '')}`); + // TODO should be handled just like a ./ or ../../../ + if (sourceValue.indexOf('../') === 0) { + // const absolutePath = path.resolve(path.dirname(packageEntryPointPath), sourceValue); + // const entry = `${dependency}/${sourceValue.replace('../', 'internal/')}`; + // const entryPath = absolutePath.replace(process.cwd(), ''); + // console.debug('it has two dot(s)!', sourceValue); + // console.debug('absolutePath???????', absolutePath); + // console.debug('ENTRY', entry); + // console.debug('PATH', entryPath); + // updateImportMap(entry, entryPath); + } else { + console.debug('it has one dot!', sourceValue); + // TODO should be handled just like a ./ or ../../../ + updateImportMap(`${dependency}/${sourceValue.replace('./', '')}`, `/node_modules/${dependency}/${sourceValue.replace('./', '')}`); + } } else { - updateImportMap(sourceValue, `/node_modules/${sourceValue}`); + // console.debug('it has NO dots!!!!'); + // updateImportMap(sourceValue, `/node_modules/${sourceValue}`); + // updateImportMap(sourceValue, `/node_modules/${sourceValue}/index.js`); } } }, ExportAllDeclaration(node) { const sourceValue = node && node.source ? node.source.value : ''; + console.debug('export all!', sourceValue); if (sourceValue !== '' && sourceValue.indexOf('http') !== 0) { if (sourceValue.indexOf('.') === 0) { + // TODO should be handled just like a ./ or ../../../ updateImportMap(`${dependency}/${sourceValue.replace('./', '')}`, `/node_modules/${dependency}/${sourceValue.replace('./', '')}`); } else { updateImportMap(sourceValue, `/node_modules/${sourceValue}`); + // TODO updateImportMap(sourceValue, `/node_modules/${sourceValue}/index.js`); } } } @@ -164,6 +199,7 @@ const walkPackageJson = async (packageJson = {}) => { : exportMapEntry.default; // use the dependency itself as an entry in the importMap + // TODO should be handled just like a ./ or ../../../ if (entry === '.') { updateImportMap(dependency, `/node_modules/${dependency}/${packageExport.replace('./', '')}`); } @@ -173,6 +209,7 @@ const walkPackageJson = async (packageJson = {}) => { } if (packageExport) { + // TODO should be handled just like a ./ or ../../../ const packageExportLocation = path.join(absoluteNodeModulesLocation, packageExport.replace('./', '')); // check all exports of an exportMap entry @@ -181,11 +218,13 @@ const walkPackageJson = async (packageJson = {}) => { const moduleContents = fs.readFileSync(packageExportLocation); await walkModule(moduleContents, dependency); + // TODO should be handled just like a ./ or ../../../ updateImportMap(`${dependency}${entry.replace('.', '')}`, `/node_modules/${dependency}/${packageExport.replace('./', '')}`); } else if (fs.lstatSync(packageExportLocation).isDirectory()) { fs.readdirSync(packageExportLocation) .filter(file => file.endsWith('.js') || file.endsWith('.mjs')) .forEach((file) => { + // TODO should be handled just like a ./ or ../../../ updateImportMap(`${dependency}/${packageExport.replace('./', '')}${file}`, `/node_modules/${dependency}/${packageExport.replace('./', '')}${file}`); }); } else { @@ -202,8 +241,10 @@ const walkPackageJson = async (packageJson = {}) => { if (fs.existsSync(packageEntryPointPath)) { const packageEntryModule = fs.readFileSync(packageEntryPointPath, 'utf-8'); - await walkModule(packageEntryModule, dependency); - updateImportMap(dependency, `/node_modules/${dependency}/${entry}`); + console.debug('1111', dependency); + await walkModule(packageEntryModule, dependency, packageEntryPointPath); + // TODO should be handled just like a ./ or ../../../ + updateImportMap(dependency, `/node_modules/${dependency}/${entry.replace('./', '')}`); await walkPackageJson(dependencyPackageJson); } } From 8713b3ee4d158a626a9064f7bbe4744cdcee0f37 Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Tue, 21 Dec 2021 09:16:56 -0500 Subject: [PATCH 02/11] refactor walkPackageJson to avoid manual relative file handling --- .../plugins/resource/plugin-node-modules.js | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js index 26b20dda0..0fce01129 100644 --- a/packages/cli/src/plugins/resource/plugin-node-modules.js +++ b/packages/cli/src/plugins/resource/plugin-node-modules.js @@ -53,8 +53,7 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { const absoluteNodeModulesLocation = await getNodeModulesLocationForPackage(dependency); console.debug('@@@@@@@@@@ ImportDeclaration', sourceValue); - // TODO should be handled just like a ./ or ../../../ - if (path.extname(sourceValue) === '' && sourceValue.indexOf('http') !== 0 && sourceValue.indexOf('./') < 0) { + if (path.extname(sourceValue) === '' && sourceValue.indexOf('http') !== 0 && sourceValue.indexOf('.') < 0) { 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 @@ -72,10 +71,9 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { } await walkPackageJson(path.join(absoluteNodeModulesLocation, 'package.json')); - // TODO should be handled just like a ./ or ../../../ - } else if (sourceValue.indexOf('./') < 0) { + } else if (sourceValue.indexOf('.') < 0) { // adding a relative import - updateImportMap(sourceValue, `/node_modules/${sourceValue}`); + // TODO delete updateImportMap(sourceValue, `/node_modules/${sourceValue}`); } else { // walk this module for all its dependencies sourceValue = sourceValue.indexOf('.js') < 0 @@ -84,8 +82,9 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { if (fs.existsSync(path.join(absoluteNodeModulesLocation, sourceValue))) { const moduleContents = fs.readFileSync(path.join(absoluteNodeModulesLocation, sourceValue)); - await walkModule(moduleContents, dependency); + await walkModule(moduleContents, dependency, packageEntryPointPath); // TODO should be handled just like a ./ or ../../../ + // console.debug('SOURCE VALUE????', sourceValue); updateImportMap(`${dependency}/${sourceValue.replace('./', '')}`, `/node_modules/${dependency}/${sourceValue.replace('./', '')}`); } } @@ -142,7 +141,8 @@ 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); + // TODO should NOT be hardcoded to process.cwd() + 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); @@ -199,9 +199,8 @@ const walkPackageJson = async (packageJson = {}) => { : exportMapEntry.default; // use the dependency itself as an entry in the importMap - // TODO should be handled just like a ./ or ../../../ 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 @@ -209,26 +208,24 @@ const walkPackageJson = async (packageJson = {}) => { } if (packageExport) { - // TODO should be handled just like a ./ or ../../../ - 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); - // TODO should be handled just like a ./ or ../../../ - updateImportMap(`${dependency}${entry.replace('.', '')}`, `/node_modules/${dependency}/${packageExport.replace('./', '')}`); + await walkModule(moduleContents, dependency, packageExportLocation); + + 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) => { - // TODO should be handled just like a ./ or ../../../ - 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)); } } } @@ -241,10 +238,9 @@ const walkPackageJson = async (packageJson = {}) => { if (fs.existsSync(packageEntryPointPath)) { const packageEntryModule = fs.readFileSync(packageEntryPointPath, 'utf-8'); - console.debug('1111', dependency); - await walkModule(packageEntryModule, dependency, packageEntryPointPath); - // TODO should be handled just like a ./ or ../../../ - updateImportMap(dependency, `/node_modules/${dependency}/${entry.replace('./', '')}`); + updateImportMap(dependency, `/node_modules/${path.join(dependency, entry)}`); + + await walkModule(packageEntryModule, dependency, packageEntryPointPath); await walkPackageJson(dependencyPackageJson); } } From 9f3d9692ab07f103e574dc56b1a22d78debfa217 Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Tue, 21 Dec 2021 14:50:25 -0500 Subject: [PATCH 03/11] refactor our final single dot path replaces --- .../plugins/resource/plugin-node-modules.js | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js index 0fce01129..da05589ed 100644 --- a/packages/cli/src/plugins/resource/plugin-node-modules.js +++ b/packages/cli/src/plugins/resource/plugin-node-modules.js @@ -42,8 +42,8 @@ const getPackageEntryPath = async (packageJson) => { }; const walkModule = async (module, dependency, packageEntryPointPath) => { - console.debug('WALK MODULE', dependency); - console.debug('packageEntryPointPath', packageEntryPointPath); + // console.debug('WALK MODULE', dependency); + // console.debug('packageEntryPointPath', packageEntryPointPath); walk.simple(acorn.parse(module, { ecmaVersion: '2020', sourceType: 'module' @@ -52,14 +52,14 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { let { value: sourceValue } = node.source; const absoluteNodeModulesLocation = await getNodeModulesLocationForPackage(dependency); - console.debug('@@@@@@@@@@ ImportDeclaration', sourceValue); + // console.debug('@@@@@@@@@@ ImportDeclaration', sourceValue); if (path.extname(sourceValue) === '' && sourceValue.indexOf('http') !== 0 && sourceValue.indexOf('.') < 0) { 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 const realLocation = path.join(absoluteNodeModulesLocation.replace(dependency, ''), `${sourceValue}/index.js`); - console.debug('realLocation', realLocation); - // TODO + // console.debug('realLocation', realLocation); + // TODO "barrel" files support if (fs.existsSync(realLocation)) { // updateImportMap(sourceValue, `/node_modules/${sourceValue}/index.js`); // const moduleContents = fs.readFileSync(realLocation, 'utf-8'); @@ -83,9 +83,8 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { if (fs.existsSync(path.join(absoluteNodeModulesLocation, sourceValue))) { const moduleContents = fs.readFileSync(path.join(absoluteNodeModulesLocation, sourceValue)); await walkModule(moduleContents, dependency, packageEntryPointPath); - // TODO should be handled just like a ./ or ../../../ - // console.debug('SOURCE VALUE????', sourceValue); - updateImportMap(`${dependency}/${sourceValue.replace('./', '')}`, `/node_modules/${dependency}/${sourceValue.replace('./', '')}`); + + updateImportMap(path.join(dependency, sourceValue), `/node_modules/${path.join(dependency, sourceValue)}`); } } }, @@ -94,7 +93,6 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { // const absoluteNodeModulesLocation = await getNodeModulesLocationForPackage(dependency); if (sourceValue !== '' && sourceValue.indexOf('http') !== 0) { - // TODO should be handled just like a ./ or ../../../ if (sourceValue.indexOf('.') === 0) { // TODO should be handled just like a ./ or ../../../ if (sourceValue.indexOf('../') === 0) { @@ -107,9 +105,8 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { // console.debug('PATH', entryPath); // updateImportMap(entry, entryPath); } else { - console.debug('it has one dot!', sourceValue); - // TODO should be handled just like a ./ or ../../../ - updateImportMap(`${dependency}/${sourceValue.replace('./', '')}`, `/node_modules/${dependency}/${sourceValue.replace('./', '')}`); + // console.debug('it has one dot!', sourceValue); + updateImportMap(path.join(dependency, sourceValue), `/node_modules/${path.join(dependency, sourceValue)}`); } } else { // console.debug('it has NO dots!!!!'); @@ -121,14 +118,12 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { ExportAllDeclaration(node) { const sourceValue = node && node.source ? node.source.value : ''; - console.debug('export all!', sourceValue); if (sourceValue !== '' && sourceValue.indexOf('http') !== 0) { if (sourceValue.indexOf('.') === 0) { - // TODO should be handled just like a ./ or ../../../ - updateImportMap(`${dependency}/${sourceValue.replace('./', '')}`, `/node_modules/${dependency}/${sourceValue.replace('./', '')}`); + updateImportMap(path.join(dependency, sourceValue), `/node_modules/${path.join(dependency, sourceValue)}`); } else { updateImportMap(sourceValue, `/node_modules/${sourceValue}`); - // TODO updateImportMap(sourceValue, `/node_modules/${sourceValue}/index.js`); + // TODO "barrel" file support - updateImportMap(sourceValue, `/node_modules/${sourceValue}/index.js`); } } } From 33ca8a78de77f0dc3fc5579f5161b92f446caf1b Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Mon, 27 Dec 2021 14:58:20 -0500 Subject: [PATCH 04/11] tweaks based on validation testing --- .../plugins/resource/plugin-node-modules.js | 33 +++++++------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js index da05589ed..3fca8e2b0 100644 --- a/packages/cli/src/plugins/resource/plugin-node-modules.js +++ b/packages/cli/src/plugins/resource/plugin-node-modules.js @@ -52,8 +52,8 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { let { value: sourceValue } = node.source; const absoluteNodeModulesLocation = await getNodeModulesLocationForPackage(dependency); - // console.debug('@@@@@@@@@@ ImportDeclaration', sourceValue); - if (path.extname(sourceValue) === '' && sourceValue.indexOf('http') !== 0 && sourceValue.indexOf('.') < 0) { + // TODO better handling of ./ check + if (path.extname(sourceValue) === '' && sourceValue.indexOf('http') !== 0 && sourceValue.indexOf('./') < 0) { 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 @@ -71,9 +71,11 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { } await walkPackageJson(path.join(absoluteNodeModulesLocation, 'package.json')); - } else if (sourceValue.indexOf('.') < 0) { - // adding a relative import - // TODO delete updateImportMap(sourceValue, `/node_modules/${sourceValue}`); + } else if (sourceValue.indexOf('./') < 0) { + // adding a bare import with extension + // TODO better handling of ./ check + updateImportMap(sourceValue, `/node_modules/${sourceValue}`); + // TODO delete or refactor with above conditional? } else { // walk this module for all its dependencies sourceValue = sourceValue.indexOf('.js') < 0 @@ -94,23 +96,10 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { if (sourceValue !== '' && sourceValue.indexOf('http') !== 0) { if (sourceValue.indexOf('.') === 0) { - // TODO should be handled just like a ./ or ../../../ - if (sourceValue.indexOf('../') === 0) { - // const absolutePath = path.resolve(path.dirname(packageEntryPointPath), sourceValue); - // const entry = `${dependency}/${sourceValue.replace('../', 'internal/')}`; - // const entryPath = absolutePath.replace(process.cwd(), ''); - // console.debug('it has two dot(s)!', sourceValue); - // console.debug('absolutePath???????', absolutePath); - // console.debug('ENTRY', entry); - // console.debug('PATH', entryPath); - // updateImportMap(entry, entryPath); - } else { - // console.debug('it has one dot!', sourceValue); - updateImportMap(path.join(dependency, sourceValue), `/node_modules/${path.join(dependency, sourceValue)}`); - } + updateImportMap(path.join(dependency, sourceValue), `/node_modules/${path.join(dependency, sourceValue)}`); } else { // console.debug('it has NO dots!!!!'); - // updateImportMap(sourceValue, `/node_modules/${sourceValue}`); + updateImportMap(sourceValue, `/node_modules/${sourceValue}`); // updateImportMap(sourceValue, `/node_modules/${sourceValue}/index.js`); } } @@ -232,10 +221,10 @@ 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'); - + updateImportMap(dependency, `/node_modules/${path.join(dependency, entry)}`); - await walkModule(packageEntryModule, dependency, packageEntryPointPath); + await walkModule(packageEntryModule, dependency, packageEntryPointPath); await walkPackageJson(dependencyPackageJson); } } From 5763e166f9f714de7ce859e4607c2b55aad4edcb Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Mon, 27 Dec 2021 17:38:44 -0500 Subject: [PATCH 05/11] refactor bare module check --- packages/cli/src/plugins/resource/plugin-node-modules.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js index 3fca8e2b0..866100e82 100644 --- a/packages/cli/src/plugins/resource/plugin-node-modules.js +++ b/packages/cli/src/plugins/resource/plugin-node-modules.js @@ -51,9 +51,11 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { 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) !== '/'; + const hasExtension = path.extname(sourceValue) !== ''; // TODO better handling of ./ check - 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 @@ -71,14 +73,14 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { } await walkPackageJson(path.join(absoluteNodeModulesLocation, 'package.json')); - } else if (sourceValue.indexOf('./') < 0) { + } else if (isBarePath) { // adding a bare import with extension // TODO better handling of ./ check updateImportMap(sourceValue, `/node_modules/${sourceValue}`); // TODO delete or refactor with above conditional? } else { // walk this module for all its dependencies - sourceValue = sourceValue.indexOf('.js') < 0 + sourceValue = !hasExtension ? `${sourceValue}.js` : sourceValue; From 98b13dbf2d6c34305b722caabd44cd12fc3bdf35 Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Tue, 28 Dec 2021 11:07:44 -0500 Subject: [PATCH 06/11] refactor walk module to read file and handle deeply relative paths for named exports --- packages/cli/package.json | 1 + .../plugins/resource/plugin-node-modules.js | 54 ++++++++----------- .../develop.default/develop.default.spec.js | 17 +++++- .../develop.default/import-map.snapshot.json | 4 +- .../test/cases/develop.default/package.json | 1 + yarn.lock | 5 ++ 6 files changed, 47 insertions(+), 35 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 75f8d1495..589ea963f 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -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", diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js index 866100e82..f80650fe2 100644 --- a/packages/cli/src/plugins/resource/plugin-node-modules.js +++ b/packages/cli/src/plugins/resource/plugin-node-modules.js @@ -24,6 +24,14 @@ const updateImportMap = (entry, entryPath) => { importMap[entry] = entryPath; }; +// 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); + + return `${dependency}${path.join(path.dirname(modulePath), specifier).replace(absoluteNodeModulesLocation, '')}`; +} + const getPackageEntryPath = async (packageJson) => { let entry = packageJson.exports ? Object.keys(packageJson.exports) // first favor export maps first @@ -41,10 +49,10 @@ const getPackageEntryPath = async (packageJson) => { return entry; }; -const walkModule = async (module, dependency, packageEntryPointPath) => { - // console.debug('WALK MODULE', dependency); - // console.debug('packageEntryPointPath', packageEntryPointPath); - 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' }), { @@ -54,28 +62,13 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { const isBarePath = sourceValue.indexOf('http') !== 0 && sourceValue.charAt(0) !== '.' && sourceValue.charAt(0) !== '/'; const hasExtension = path.extname(sourceValue) !== ''; - // TODO better handling of ./ check 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 - const realLocation = path.join(absoluteNodeModulesLocation.replace(dependency, ''), `${sourceValue}/index.js`); - // console.debug('realLocation', realLocation); - // TODO "barrel" files support - if (fs.existsSync(realLocation)) { - // updateImportMap(sourceValue, `/node_modules/${sourceValue}/index.js`); - // const moduleContents = fs.readFileSync(realLocation, 'utf-8'); - // console.debug('going deep!!!!'); - // await walkModule(moduleContents, dependency); - } else { - updateImportMap(sourceValue, `/node_modules/${sourceValue}`); - } + updateImportMap(sourceValue, `/node_modules/${sourceValue}`); } await walkPackageJson(path.join(absoluteNodeModulesLocation, 'package.json')); } else if (isBarePath) { - // adding a bare import with extension - // TODO better handling of ./ check updateImportMap(sourceValue, `/node_modules/${sourceValue}`); // TODO delete or refactor with above conditional? } else { @@ -85,8 +78,7 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { : sourceValue; if (fs.existsSync(path.join(absoluteNodeModulesLocation, sourceValue))) { - const moduleContents = fs.readFileSync(path.join(absoluteNodeModulesLocation, sourceValue)); - await walkModule(moduleContents, dependency, packageEntryPointPath); + await walkModule(path.join(absoluteNodeModulesLocation, sourceValue), dependency); updateImportMap(path.join(dependency, sourceValue), `/node_modules/${path.join(dependency, sourceValue)}`); } @@ -94,15 +86,16 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { }, async ExportNamedDeclaration(node) { const sourceValue = node && node.source ? node.source.value : ''; - // const absoluteNodeModulesLocation = await getNodeModulesLocationForPackage(dependency); if (sourceValue !== '' && sourceValue.indexOf('http') !== 0) { + // handle relative specifier if (sourceValue.indexOf('.') === 0) { - updateImportMap(path.join(dependency, sourceValue), `/node_modules/${path.join(dependency, sourceValue)}`); + const entry = `/node_modules/${await resolveRelativeSpecifier(sourceValue, modulePath, dependency)}`; + + updateImportMap(path.join(dependency, sourceValue), entry); } else { - // console.debug('it has NO dots!!!!'); + // handle bare specifier updateImportMap(sourceValue, `/node_modules/${sourceValue}`); - // updateImportMap(sourceValue, `/node_modules/${sourceValue}/index.js`); } } }, @@ -114,7 +107,6 @@ const walkModule = async (module, dependency, packageEntryPointPath) => { updateImportMap(path.join(dependency, sourceValue), `/node_modules/${path.join(dependency, sourceValue)}`); } else { updateImportMap(sourceValue, `/node_modules/${sourceValue}`); - // TODO "barrel" file support - updateImportMap(sourceValue, `/node_modules/${sourceValue}/index.js`); } } } @@ -199,9 +191,7 @@ const walkPackageJson = async (packageJson = {}) => { // 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, packageExportLocation); + await walkModule(packageExportLocation, dependency); updateImportMap(path.join(dependency, entry), `/node_modules/${path.join(dependency, packageExport)}`); } else if (fs.lstatSync(packageExportLocation).isDirectory()) { @@ -222,11 +212,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'); - updateImportMap(dependency, `/node_modules/${path.join(dependency, entry)}`); - await walkModule(packageEntryModule, dependency, packageEntryPointPath); + await walkModule(packageEntryPointPath, dependency); await walkPackageJson(dependencyPackageJson); } } diff --git a/packages/cli/test/cases/develop.default/develop.default.spec.js b/packages/cli/test/cases/develop.default/develop.default.spec.js index 46ac1a452..4d715f709 100644 --- a/packages/cli/test/cases/develop.default/develop.default.spec.js +++ b/packages/cli/test/cases/develop.default/develop.default.spec.js @@ -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`); @@ -437,7 +449,10 @@ describe('Develop Greenwood With: ', function() { ...materialThemePackageJson, ...materialThemeLibs, ...tslibPackageJson, - ...tslibLibs + ...tslibLibs, + ...stencilCorePackageJson, + ...stencilCoreCoreLibs, + ...stencilCoreClientLibs ]); return new Promise(async (resolve) => { diff --git a/packages/cli/test/cases/develop.default/import-map.snapshot.json b/packages/cli/test/cases/develop.default/import-map.snapshot.json index 7f37c15f2..b7d3ddf55 100644 --- a/packages/cli/test/cases/develop.default/import-map.snapshot.json +++ b/packages/cli/test/cases/develop.default/import-map.snapshot.json @@ -270,5 +270,7 @@ "@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" } \ No newline at end of file diff --git a/packages/cli/test/cases/develop.default/package.json b/packages/cli/test/cases/develop.default/package.json index d148bd158..cb0f93aca 100644 --- a/packages/cli/test/cases/develop.default/package.json +++ b/packages/cli/test/cases/develop.default/package.json @@ -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" } diff --git a/yarn.lock b/yarn.lock index 5fd2fca1d..d21c527dc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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" From 52b3ead8da2727d34134613f16b76e5ccfd6f4ed Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Tue, 28 Dec 2021 11:20:40 -0500 Subject: [PATCH 07/11] apply relative path handling across all AST visitor handlers --- .../cli/src/plugins/resource/plugin-node-modules.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js index f80650fe2..b2174b3d0 100644 --- a/packages/cli/src/plugins/resource/plugin-node-modules.js +++ b/packages/cli/src/plugins/resource/plugin-node-modules.js @@ -70,7 +70,6 @@ const walkModule = async (modulePath, dependency) => { await walkPackageJson(path.join(absoluteNodeModulesLocation, 'package.json')); } else if (isBarePath) { updateImportMap(sourceValue, `/node_modules/${sourceValue}`); - // TODO delete or refactor with above conditional? } else { // walk this module for all its dependencies sourceValue = !hasExtension @@ -78,9 +77,10 @@ const walkModule = async (modulePath, dependency) => { : sourceValue; if (fs.existsSync(path.join(absoluteNodeModulesLocation, sourceValue))) { + const entry = `/node_modules/${await resolveRelativeSpecifier(sourceValue, modulePath, dependency)}`; await walkModule(path.join(absoluteNodeModulesLocation, sourceValue), dependency); - updateImportMap(path.join(dependency, sourceValue), `/node_modules/${path.join(dependency, sourceValue)}`); + updateImportMap(path.join(dependency, sourceValue), entry); } } }, @@ -99,12 +99,14 @@ const walkModule = async (modulePath, dependency) => { } } }, - ExportAllDeclaration(node) { + async ExportAllDeclaration(node) { const sourceValue = node && node.source ? node.source.value : ''; if (sourceValue !== '' && sourceValue.indexOf('http') !== 0) { if (sourceValue.indexOf('.') === 0) { - updateImportMap(path.join(dependency, sourceValue), `/node_modules/${path.join(dependency, sourceValue)}`); + const entry = `/node_modules/${await resolveRelativeSpecifier(sourceValue, modulePath, dependency)}`; + + updateImportMap(path.join(dependency, sourceValue), entry); } else { updateImportMap(sourceValue, `/node_modules/${sourceValue}`); } From 98e27fd2e4de250d167d984de7294d061642aee1 Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Tue, 28 Dec 2021 18:07:55 -0500 Subject: [PATCH 08/11] dont walk packages that already provide export maps --- packages/cli/src/plugins/resource/plugin-node-modules.js | 8 +++----- .../test/cases/develop.default/import-map.snapshot.json | 6 ------ 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js index b2174b3d0..e84b150b1 100644 --- a/packages/cli/src/plugins/resource/plugin-node-modules.js +++ b/packages/cli/src/plugins/resource/plugin-node-modules.js @@ -59,7 +59,7 @@ const walkModule = async (modulePath, dependency) => { 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) !== '/'; + const isBarePath = sourceValue.indexOf('http') !== 0 && sourceValue.charAt(0) !== '.' && sourceValue.charAt(0) !== path.sep; const hasExtension = path.extname(sourceValue) !== ''; if (isBarePath && !hasExtension) { @@ -152,7 +152,7 @@ const walkPackageJson = async (packageJson = {}) => { break; case 'object': const entryTypes = Object.keys(mapItem); - + if (entryTypes.import) { esmPath = entryTypes.import; } else if (entryTypes.require) { @@ -186,15 +186,13 @@ const walkPackageJson = async (packageJson = {}) => { // is probably a file, so _not_ an export array, package.json, or wildcard export packageExport = exportMapEntry; } - + if (packageExport) { 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')) { - await walkModule(packageExportLocation, dependency); - updateImportMap(path.join(dependency, entry), `/node_modules/${path.join(dependency, packageExport)}`); } else if (fs.lstatSync(packageExportLocation).isDirectory()) { fs.readdirSync(packageExportLocation) diff --git a/packages/cli/test/cases/develop.default/import-map.snapshot.json b/packages/cli/test/cases/develop.default/import-map.snapshot.json index b7d3ddf55..ad256ef39 100644 --- a/packages/cli/test/cases/develop.default/import-map.snapshot.json +++ b/packages/cli/test/cases/develop.default/import-map.snapshot.json @@ -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", @@ -258,18 +255,15 @@ "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", "@stencil/core": "/node_modules/@stencil/core/internal/stencil-core/index.js", "@stencil/client/index.js": "/node_modules/@stencil/core/internal/client/index.js" From 26037ffea9ea4d23bdf140f87a57e1ab984f6029 Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Tue, 28 Dec 2021 18:08:23 -0500 Subject: [PATCH 09/11] dont walk packages that already provide export maps --- packages/cli/src/plugins/resource/plugin-node-modules.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js index e84b150b1..129b8a775 100644 --- a/packages/cli/src/plugins/resource/plugin-node-modules.js +++ b/packages/cli/src/plugins/resource/plugin-node-modules.js @@ -190,8 +190,6 @@ const walkPackageJson = async (packageJson = {}) => { if (packageExport) { 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')) { updateImportMap(path.join(dependency, entry), `/node_modules/${path.join(dependency, packageExport)}`); } else if (fs.lstatSync(packageExportLocation).isDirectory()) { From c209a60da6478a53793fbf916c7480bc0209d9d0 Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Sat, 26 Feb 2022 09:26:58 -0800 Subject: [PATCH 10/11] force ESM style path separators --- packages/cli/src/plugins/resource/plugin-node-modules.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js index 129b8a775..88f2ff61c 100644 --- a/packages/cli/src/plugins/resource/plugin-node-modules.js +++ b/packages/cli/src/plugins/resource/plugin-node-modules.js @@ -21,7 +21,8 @@ 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' @@ -29,7 +30,8 @@ const updateImportMap = (entry, entryPath) => { async function resolveRelativeSpecifier(specifier, modulePath, dependency) { const absoluteNodeModulesLocation = await getNodeModulesLocationForPackage(dependency); - return `${dependency}${path.join(path.dirname(modulePath), specifier).replace(absoluteNodeModulesLocation, '')}`; + // 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) => { From 459235d654cba712dd1ca72f5934157eb8b48e71 Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Wed, 2 Mar 2022 21:22:58 -0500 Subject: [PATCH 11/11] tracked TODO item --- packages/cli/src/plugins/resource/plugin-node-modules.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/plugins/resource/plugin-node-modules.js b/packages/cli/src/plugins/resource/plugin-node-modules.js index 88f2ff61c..b6f31a61d 100644 --- a/packages/cli/src/plugins/resource/plugin-node-modules.js +++ b/packages/cli/src/plugins/resource/plugin-node-modules.js @@ -123,7 +123,6 @@ const walkPackageJson = async (packageJson = {}) => { // and walk its package.json for its dependencies for (const dependency of Object.keys(packageJson.dependencies || {})) { - // TODO should NOT be hardcoded to process.cwd() 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'));