Skip to content

Commit

Permalink
fix: aws-cdk-lib imports from ESM modules are broken (#23846)
Browse files Browse the repository at this point in the history
PR #23813 made imports lazy, but in the resulting code, Nodejs no longer recognizes the exports when importing `aws-cdk-lib` from an ESM module.

To solve this, vend two different index files: one for use by CJS imports, one for use by ESM imports.

ESM modules will still try to load the entire library, so they don't benefit from the speed boost. This is unavoidable: we tried a more complex method that forced ESM to recognize the lazy module references anyway (by tricking the backwards compatibility lexer), but ESM did not experience a speed boost, indicating that it was crawling the entire module irrespective of the submodule accessor's laziness. So, we are opting for the simpler solution of vending two index files instead.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Jan 26, 2023
1 parent dd21069 commit cf2e498
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 22 deletions.
3 changes: 3 additions & 0 deletions packages/aws-cdk-lib/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
!NOTICE
!README.md
!scripts/
!scripts/*.ts
!scripts/*.sh
scripts/*.d.ts

.LAST_BUILD
*.snk
Expand Down
7 changes: 2 additions & 5 deletions packages/aws-cdk-lib/.npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ coverage
# Build gear
build-tools
dist
scripts
.LAST_BUILD
.LAST_PACKAGE

Expand All @@ -23,6 +24,7 @@ tsconfig.json
!.jsii
!.jsii.gz
.eslintrc.js

# exclude cdk artifacts
**/cdk.out
junit.xml
Expand All @@ -31,8 +33,3 @@ junit.xml

# exclude source maps as they only work locally
*.map

# exclude tempory export files
exports.d.ts
exports.js
exports.js.map
15 changes: 13 additions & 2 deletions packages/aws-cdk-lib/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
"stripDeprecated": true,
"compressAssembly": true,
"post": [
"cp exports.js index.js",
"node ./scripts/verify-imports-resolve-same.js",
"node ./scripts/verify-imports-shielded.js",
"/bin/bash ./scripts/minify-sources.sh"
Expand Down Expand Up @@ -407,7 +406,10 @@
"excludeExperimentalModules": true
},
"exports": {
".": "./index.js",
".": {
"import": "./index.js",
"require": "./lazy-index.js"
},
"./package.json": "./package.json",
"./.jsii": "./.jsii",
"./.warnings.jsii.js": "./.warnings.jsii.js",
Expand Down Expand Up @@ -481,6 +483,7 @@
"./aws-dlm": "./aws-dlm/index.js",
"./aws-dms": "./aws-dms/index.js",
"./aws-docdb": "./aws-docdb/index.js",
"./aws-docdbelastic": "./aws-docdbelastic/index.js",
"./aws-dynamodb": "./aws-dynamodb/index.js",
"./aws-ec2": "./aws-ec2/index.js",
"./aws-ecr": "./aws-ecr/index.js",
Expand Down Expand Up @@ -513,6 +516,7 @@
"./aws-globalaccelerator": "./aws-globalaccelerator/index.js",
"./aws-globalaccelerator-endpoints": "./aws-globalaccelerator-endpoints/index.js",
"./aws-glue": "./aws-glue/index.js",
"./aws-grafana": "./aws-grafana/index.js",
"./aws-greengrass": "./aws-greengrass/index.js",
"./aws-greengrassv2": "./aws-greengrassv2/index.js",
"./aws-groundstation": "./aws-groundstation/index.js",
Expand All @@ -537,6 +541,7 @@
"./aws-ivs": "./aws-ivs/index.js",
"./aws-kafkaconnect": "./aws-kafkaconnect/index.js",
"./aws-kendra": "./aws-kendra/index.js",
"./aws-kendraranking": "./aws-kendraranking/index.js",
"./aws-kinesis": "./aws-kinesis/index.js",
"./aws-kinesisanalytics": "./aws-kinesisanalytics/index.js",
"./aws-kinesisanalyticsv2": "./aws-kinesisanalyticsv2/index.js",
Expand Down Expand Up @@ -573,13 +578,17 @@
"./aws-networkfirewall": "./aws-networkfirewall/index.js",
"./aws-networkmanager": "./aws-networkmanager/index.js",
"./aws-nimblestudio": "./aws-nimblestudio/index.js",
"./aws-oam": "./aws-oam/index.js",
"./aws-opensearchserverless": "./aws-opensearchserverless/index.js",
"./aws-opensearchservice": "./aws-opensearchservice/index.js",
"./aws-opsworks": "./aws-opsworks/index.js",
"./aws-opsworkscm": "./aws-opsworkscm/index.js",
"./aws-organizations": "./aws-organizations/index.js",
"./aws-panorama": "./aws-panorama/index.js",
"./aws-personalize": "./aws-personalize/index.js",
"./aws-pinpoint": "./aws-pinpoint/index.js",
"./aws-pinpointemail": "./aws-pinpointemail/index.js",
"./aws-pipes": "./aws-pipes/index.js",
"./aws-qldb": "./aws-qldb/index.js",
"./aws-quicksight": "./aws-quicksight/index.js",
"./aws-ram": "./aws-ram/index.js",
Expand All @@ -589,6 +598,7 @@
"./aws-refactorspaces": "./aws-refactorspaces/index.js",
"./aws-rekognition": "./aws-rekognition/index.js",
"./aws-resiliencehub": "./aws-resiliencehub/index.js",
"./aws-resourceexplorer2": "./aws-resourceexplorer2/index.js",
"./aws-resourcegroups": "./aws-resourcegroups/index.js",
"./aws-robomaker": "./aws-robomaker/index.js",
"./aws-rolesanywhere": "./aws-rolesanywhere/index.js",
Expand All @@ -607,6 +617,7 @@
"./aws-s3outposts": "./aws-s3outposts/index.js",
"./aws-sagemaker": "./aws-sagemaker/index.js",
"./aws-sam": "./aws-sam/index.js",
"./aws-scheduler": "./aws-scheduler/index.js",
"./aws-sdb": "./aws-sdb/index.js",
"./aws-secretsmanager": "./aws-secretsmanager/index.js",
"./aws-securityhub": "./aws-securityhub/index.js",
Expand Down
3 changes: 1 addition & 2 deletions packages/aws-cdk-lib/scripts/minify-sources.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
scriptdir=$(cd $(dirname $0) && pwd)
cd ${scriptdir}/..

find . -name '*.js' ! -name 'exports.js' ! -name '.eslintrc.js' ! -path '*node_modules*' | xargs npx esbuild \
--sourcemap \
find . -name '*.js' ! -name '.eslintrc.js' ! -path '*node_modules*' | xargs npx esbuild \
--platform=node \
--format=cjs \
--minify-whitespace \
Expand Down
47 changes: 34 additions & 13 deletions tools/@aws-cdk/ubergen/bin/ubergen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ interface LibraryReference {
readonly shortName: string;
}

type Export = string | {
readonly import?: string;
readonly require?: string;
};

interface PackageJson {
readonly main?: string;
readonly description?: string;
Expand Down Expand Up @@ -103,7 +108,7 @@ interface PackageJson {
*/
readonly exports?: Record<string, string>;
};
exports?: Record<string, string>;
exports?: Record<string, Export>;
}

/**
Expand Down Expand Up @@ -269,7 +274,10 @@ async function prepareSourceFiles(libraries: readonly LibraryReference[], packag
// Control 'exports' field of the 'package.json'. This will control what kind of 'import' statements are
// allowed for this package: we only want to allow the exact import statements that we want to support.
packageJson.exports = {
'.': './index.js',
'.': {
import: './index.js',
require: './lazy-index.js',
},

// We need to expose 'package.json' and '.jsii' because 'jsii' and 'jsii-reflect' load them using
// require(). (-_-). Can be removed after https://github.com/aws/jsii/pull/3205 gets merged.
Expand All @@ -281,12 +289,15 @@ async function prepareSourceFiles(libraries: readonly LibraryReference[], packag
};

// We use the index.ts to compile type definitions.
// At the very end we replace the compiled index.js file with our fixed version exports.js.
// exports.js has the top level submodules exports defined as a getter function,
// so they are not automatically loaded when importing from `aws-cdk-lib`.
//
// We build two indexes: one for eager loading (used by ESM modules), and one
// for lazy loading (used by CJS modules). The lazy loading will result in faster
// loading times, because we don't have to load and parse all submodules right away,
// but is not compatible with ESM's loading algorithm.
//
// This improves AWS CDK app performance by ~400ms.
const indexStatements = new Array<string>();
const exportsStatements = new Array<string>();
const lazyExports = new Array<string>();

for (const library of libraries) {
const libDir = path.join(libRoot, library.shortName);
Expand All @@ -297,19 +308,21 @@ async function prepareSourceFiles(libraries: readonly LibraryReference[], packag
}
if (library.shortName === 'core') {
indexStatements.push(`export * from './${library.shortName}';`);
exportsStatements.unshift(`export * from './${library.shortName}';`);
lazyExports.unshift(`export * from './${library.shortName}';`);
} else {
indexStatements.push(`export * as ${library.shortName.replace(/-/g, '_')} from './${library.shortName}';`);
exportsStatements.push(`Object.defineProperty(exports, '${library.shortName.replace(/-/g, '_')}', { get: function () { return require('./${library.shortName}'); } });`);
const exportName = library.shortName.replace(/-/g, '_');

indexStatements.push(`export * as ${exportName} from './${library.shortName}';`);
lazyExports.push(`Object.defineProperty(exports, '${exportName}', { get: function () { return require('./${library.shortName}'); } });`);
}
copySubmoduleExports(packageJson.exports, library, library.shortName);
}

// make the exports.ts file pass linting
exportsStatements.unshift('/* eslint-disable @typescript-eslint/no-require-imports */');
lazyExports.unshift('/* eslint-disable @typescript-eslint/no-require-imports */');

await fs.writeFile(path.join(libRoot, 'index.ts'), indexStatements.join('\n'), { encoding: 'utf8' });
await fs.writeFile(path.join(libRoot, 'exports.ts'), exportsStatements.join('\n'), { encoding: 'utf8' });
await fs.writeFile(path.join(libRoot, 'lazy-index.ts'), lazyExports.join('\n'), { encoding: 'utf8' });

console.log('\t🍺 Success!');
}
Expand All @@ -320,13 +333,21 @@ async function prepareSourceFiles(libraries: readonly LibraryReference[], packag
* Replace the original 'main' export with an export of the new '<submodule>/index.ts` file we've written
* in 'transformPackage'.
*/
function copySubmoduleExports(targetExports: Record<string, string>, library: LibraryReference, subdirectory: string) {
function copySubmoduleExports(targetExports: Record<string, Export>, library: LibraryReference, subdirectory: string) {
const visibleName = library.shortName;

// Do both REAL "exports" section, as well as virtual, ubergen-only "exports" section
for (const exportSet of [library.packageJson.exports, library.packageJson.ubergen?.exports]) {
for (const [relPath, relSource] of Object.entries(exportSet ?? {})) {
targetExports[`./${unixPath(path.join(visibleName, relPath))}`] = `./${unixPath(path.join(subdirectory, relSource))}`;
targetExports[`./${unixPath(path.join(visibleName, relPath))}`] = resolveExport(relSource);
}
}

function resolveExport<A extends Export>(exp: A): A {
if (typeof exp === 'string') {
return `./${unixPath(path.join(subdirectory, exp))}` as any;
} else {
return Object.fromEntries(Object.entries(exp).map(([k, v]) => [k, v ? resolveExport(v) : undefined])) as any;
}
}

Expand Down

0 comments on commit cf2e498

Please sign in to comment.