diff --git a/.pullapprove.yml b/.pullapprove.yml index 455dabecdff..91a6c1e2b6f 100644 --- a/.pullapprove.yml +++ b/.pullapprove.yml @@ -1170,7 +1170,7 @@ groups: 'tools/bazel-repo-patches/**/{*,.*}', 'tools/circular_dependency_test/**/{*,.*}', 'tools/contributing-stats/**/{*,.*}', - 'tools/esm-loader/**/{*,.*}', + 'tools/esm-interop/**/{*,.*}', 'tools/gulp-tasks/**/{*,.*}', 'tools/legacy-saucelabs/**/{*,.*}', 'tools/rxjs/**/{*,.*}', diff --git a/third_party/github.com/lukeed/resolve.exports/BUILD.bazel b/third_party/github.com/lukeed/resolve.exports/BUILD.bazel index 75290200201..c9202b9fa06 100644 --- a/third_party/github.com/lukeed/resolve.exports/BUILD.bazel +++ b/third_party/github.com/lukeed/resolve.exports/BUILD.bazel @@ -11,6 +11,6 @@ filegroup( "index.mjs", ], visibility = [ - "//tools/esm-loader:__pkg__", + "//tools/esm-interop:__pkg__", ], ) diff --git a/tools/defaults.bzl b/tools/defaults.bzl index e7de068ed44..009173ed1f1 100644 --- a/tools/defaults.bzl +++ b/tools/defaults.bzl @@ -8,7 +8,6 @@ load("@npm//@bazel/rollup:index.bzl", _rollup_bundle = "rollup_bundle") load("@npm//@bazel/terser:index.bzl", "terser_minified") load("@npm//@bazel/protractor:index.bzl", _protractor_web_test_suite = "protractor_web_test_suite") load("@npm//typescript:index.bzl", "tsc") -load("//packages/bazel:index.bzl", _ng_module = "ng_module", _ng_package = "ng_package") load("@npm//@angular/build-tooling/bazel/app-bundling:index.bzl", _app_bundle = "app_bundle") load("@npm//@angular/build-tooling/bazel/http-server:index.bzl", _http_server = "http_server") load("@npm//@angular/build-tooling/bazel/karma:index.bzl", _karma_web_test = "karma_web_test", _karma_web_test_suite = "karma_web_test_suite") @@ -17,6 +16,8 @@ load("@npm//@angular/build-tooling/bazel:extract_js_module_output.bzl", "extract load("@npm//@angular/build-tooling/bazel:extract_types.bzl", _extract_types = "extract_types") load("@npm//@angular/build-tooling/bazel/esbuild:index.bzl", _esbuild = "esbuild", _esbuild_config = "esbuild_config") load("@npm//tsec:index.bzl", _tsec_test = "tsec_test") +load("//packages/bazel:index.bzl", _ng_module = "ng_module", _ng_package = "ng_package") +load("//tools/esm-interop:index.bzl", "enable_esm_node_module_loader", "extract_esm_outputs", "install_esm_loaders") _DEFAULT_TSCONFIG_TEST = "//packages:tsconfig-test" _INTERNAL_NG_MODULE_COMPILER = "//packages/bazel/src/ngc-wrapped" @@ -382,10 +383,19 @@ def protractor_web_test_suite(**kwargs): **kwargs ) -def nodejs_binary(data = [], env = {}, templated_args = [], enable_linker = False, **kwargs): - data = data + [ - "@%s//source-map-support" % _node_modules_workspace_name(), - ] +def nodejs_binary( + name, + entry_point, + testonly = False, + data = [], + env = {}, + templated_args = [], + enable_linker = False, + **kwargs): + npm_workspace = _node_modules_workspace_name() + rule_data = [] + + (templated_args, rule_data) = install_esm_loaders(templated_args, rule_data) if not enable_linker: templated_args = templated_args + [ @@ -394,16 +404,30 @@ def nodejs_binary(data = [], env = {}, templated_args = [], enable_linker = Fals "--nobazel_run_linker", ] - (env, templated_args, data) = _apply_esm_import_patch(env, templated_args, data) + env = enable_esm_node_module_loader(npm_workspace, env) + + extract_esm_outputs( + name = "%s_esm_deps" % name, + testonly = testonly, + deps = data, + ) _nodejs_binary( - data = data, + name = name, + data = [":%s_esm_deps" % name] + rule_data, + testonly = testonly, + entry_point = entry_point.replace(".js", ".mjs"), env = env, templated_args = templated_args, + use_esm = True, **kwargs ) def nodejs_test(data = [], env = {}, templated_args = [], enable_linker = False, **kwargs): + rule_data = [] + + (templated_args, rule_data) = install_esm_loaders(templated_args, rule_data) + if not enable_linker: templated_args = templated_args + [ # Disable the linker and rely on patched resolution which works better on Windows @@ -411,10 +435,11 @@ def nodejs_test(data = [], env = {}, templated_args = [], enable_linker = False, "--nobazel_run_linker", ] - (env, templated_args, data) = _apply_esm_import_patch(env, templated_args, data) + npm_workspace = _node_modules_workspace_name() + env = enable_esm_node_module_loader(npm_workspace, env) _nodejs_test( - data = data, + data = data + rule_data, env = env, templated_args = templated_args, **kwargs @@ -423,23 +448,6 @@ def nodejs_test(data = [], env = {}, templated_args = [], enable_linker = False, def _node_modules_workspace_name(): return "npm" if not native.package_name().startswith("aio") else "aio_npm" -def _apply_esm_import_patch(env, templated_args, data): - """Adjust properties from a nodejs_binary/test to provide a custom esm loader - to resolve third-party deps. Unlike for cjs modules, rules_nodejs doesn't patch - imports when the linker is disabled.""" - - env = dict(env, **{"NODE_MODULES_WORKSPACE_NAME": _node_modules_workspace_name()}) - templated_args = templated_args + [ - "--node_options=--loader=file:///$$(rlocation $(rootpath //tools/esm-loader:esm-loader.mjs))", - "--node_options=--no-warnings", # `--loader` is an experimental feature with warnings. - ] - data = data + [ - "//tools/esm-loader", - "//tools/esm-loader:esm-loader.mjs", - ] - - return (env, templated_args, data) - def npm_package_bin(args = [], **kwargs): _npm_package_bin( # Disable the linker and rely on patched resolution which works better on Windows diff --git a/tools/esm-interop/BUILD.bazel b/tools/esm-interop/BUILD.bazel index 1cad7ffcb5c..b5d85c7f55c 100644 --- a/tools/esm-interop/BUILD.bazel +++ b/tools/esm-interop/BUILD.bazel @@ -1,13 +1,15 @@ package(default_visibility = ["//visibility:public"]) exports_files([ - "esm-loader.mjs", + "esm-main-loader.mjs", ]) filegroup( - name = "esm-loader", + name = "loaders", srcs = [ - "esm-loader.mjs", + "esm-extension-loader.mjs", + "esm-main-loader.mjs", + "esm-node-module-loader.mjs", "//third_party/github.com/lukeed/resolve.exports", ], ) diff --git a/tools/esm-interop/esm-extension-loader.mjs b/tools/esm-interop/esm-extension-loader.mjs new file mode 100644 index 00000000000..a1f1672d2fb --- /dev/null +++ b/tools/esm-interop/esm-extension-loader.mjs @@ -0,0 +1,59 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +const explicitExtensionRe = /\.[mc]?js$/; +const explicitJsExtensionRe = /\.js$/; + +/* + * NodeJS resolver that enables the interop with the current Bazel setup. + * + * The loader will attempt resolution by replacing explicit extension with + * their ESM variants. It also tries completing import specifiers in case no + * extension of explicit file is specified. + * + * There are a few cases: + * + * * Relative imports without an extension. This happens because our + * source files cannot use explicit `.mjs` extensions yet. + * * Relative imports with an explicit extension to `.js`. This may + * be generated by TypeScript as we have `.ts` source files. + * * Local module imports. In NPM, extensions are not needed as the + * `package.json` `exports` help resolving. In Bazel when dealing with + * 1st-party packages- `package.json` is not consulted in resolution. + * 1. The NPM artifacts differ from the source compilation output. + * 2. It results in additional churn, having to put `package.json` into `bin`. + */ +export async function resolve(specifier, context, nextResolve) { + // Actual resolution is the actual specifier. This is where errors + // should not be silenced as it's the actual user-specifier. + let resolveError = null; + try { + return await nextResolve(specifier, context); + } catch (e) { + resolveError = e; + } + + const interopAttempts = []; + if (explicitJsExtensionRe.test(specifier)) { + interopAttempts.push(specifier.replace(explicitJsExtensionRe, '.mjs')); + } + + if (!explicitExtensionRe.test(specifier)) { + interopAttempts.push(`${specifier}.mjs`); + interopAttempts.push(`${specifier}/index.mjs`); + } + + for (const attempt of interopAttempts) { + try { + return await nextResolve(attempt, context); + } catch {} + } + + // Rethrow the existing resolution error. + throw resolveError; +} diff --git a/tools/esm-interop/esm-loaders.bzl b/tools/esm-interop/esm-loaders.bzl new file mode 100644 index 00000000000..952b31e89b1 --- /dev/null +++ b/tools/esm-interop/esm-loaders.bzl @@ -0,0 +1,26 @@ +"""ESM loader helpers.""" + +def install_esm_loaders( + templated_args, + data): + """Installs a NodeJS import loader for ESM support. Individual loades may \ + be controlled via environment variables. + + Args: + templated_args: Existing list of arguments passed to the binary/test. + data: Existing runtime dependencies for the binary/test. + + Returns: + A ruple with the updated `templated_args` and `data`. + """ + + templated_args = templated_args + [ + "--node_options=--experimental-loader=file:///$$(rlocation $(rootpath //tools/esm-interop:esm-main-loader.mjs))", + "--node_options=--no-warnings", # `--loader` is an experimental feature with warnings. + ] + data = data + [ + "//tools/esm-interop:esm-main-loader.mjs", + "//tools/esm-interop:loaders", + ] + + return (templated_args, data) diff --git a/tools/esm-interop/esm-main-loader.mjs b/tools/esm-interop/esm-main-loader.mjs new file mode 100644 index 00000000000..45b3df2def6 --- /dev/null +++ b/tools/esm-interop/esm-main-loader.mjs @@ -0,0 +1,39 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {extname} from 'path'; +import {fileURLToPath} from 'url'; + +import * as extensionLoader from './esm-extension-loader.mjs'; +import * as nodeModuleLoader from './esm-node-module-loader.mjs'; + +const loaders = [extensionLoader, nodeModuleLoader]; + +export async function resolve(initialSpecifier, initialCtx, defaultResolve) { + let nextFn = (i) => (s, c) => { + if (i === loaders.length) { + return defaultResolve(s, c, defaultResolve); + } + return loaders[i].resolve(s, c, nextFn(i + 1)); + }; + + return nextFn(0)(initialSpecifier, initialCtx); +} + +export async function load(url, context, defaultLoad) { + // Using `--loader` causes non-ESM extension-less files like + // for `typescript/bin/tsc` to be considered as ESM. This is a bug + // via: https://github.com/nodejs/node/issues/33226. + // Workaround is to load such extension-less files as CommonJS. Similar + // to how they are loaded without `--loader` being specified. + if (url.startsWith('file://') && extname(fileURLToPath(url)) === '') { + context.format = 'commonjs'; + } + + return defaultLoad(url, context, defaultLoad); +} diff --git a/tools/esm-interop/esm-node-module-loader.bzl b/tools/esm-interop/esm-node-module-loader.bzl new file mode 100644 index 00000000000..0cb29e4549d --- /dev/null +++ b/tools/esm-interop/esm-node-module-loader.bzl @@ -0,0 +1,22 @@ +"""ESM node module loader helpers.""" + +def enable_esm_node_module_loader( + node_modules_workspace, + env): + """Enables a NodeJS import loader that ensures modules can be resolved from the Bazel repository. + + Args: + node_modules_workspace: Name of the workspace in which node modules are available. + env: Struct of environment variables passed to a binary/test. + + Returns: + The updated `env` dictionary. + """ + + env = dict( + env, + NODE_MODULES_WORKSPACE_NAME = node_modules_workspace, + ESM_NODE_MODULE_LOADER_ENABLED = "true", + ) + + return env diff --git a/tools/esm-interop/esm-node-module-loader.mjs b/tools/esm-interop/esm-node-module-loader.mjs index dba5ab49421..95f1d4d266b 100644 --- a/tools/esm-interop/esm-node-module-loader.mjs +++ b/tools/esm-interop/esm-node-module-loader.mjs @@ -8,47 +8,61 @@ import fs from 'fs'; import path from 'path'; +import {createRequire} from 'module'; import {pathToFileURL} from 'url'; import {resolve as resolveExports} from '../../third_party/github.com/lukeed/resolve.exports/index.mjs'; +// The Bazel NodeJS rules patch `require` to support first-party +// mapped packages. We cannot replicate this logic without patching +// the Bazel rules, so instead we leverage the existing `require` +// patched function as it knows about first party mapped packages. +const requireFn = createRequire(import.meta.url); + +const npmDepsWorkspace = process.env.NODE_MODULES_WORKSPACE_NAME; +const runfilesRoot = path.resolve(process.env.RUNFILES); +const nodeModulesPath = path.join(runfilesRoot, npmDepsWorkspace, 'node_modules'); + /* - Custom module loader (see https://nodejs.org/api/cli.html#--experimental-loadermodule) to support - loading third-party packages in esm modules when the rules_nodejs linker is disabled. Resolves - third-party imports from the node_modules folder in the bazel workspace defined by - process.env.NODE_MODULES_WORKSPACE_NAME, and uses default resolution for all other imports. - - This is required because rules_nodejs only patches requires in cjs modules when the linker - is disabled, not imports in mjs modules. + Custom module loader to support loading 1st-party and 3rd-party node + modules when the linker is disabled. This is required because `rules_nodejs` + only patches requires in cjs modules when the linker is disabled. */ -export async function resolve(specifier, context, defaultResolve) { +export async function resolve(specifier, context, nextResolve) { + // Only activate this loader when explicitly enabled. + if (process.env.ESM_NODE_MODULE_LOADER_ENABLED !== 'true') { + return nextResolve(specifier, context); + } + if (!isNodeOrNpmPackageImport(specifier)) { - return defaultResolve(specifier, context, defaultResolve); + return nextResolve(specifier, context); } - const runfilesRoot = path.resolve(process.env.RUNFILES); - const nodeModules = path.join( - runfilesRoot, - process.env.NODE_MODULES_WORKSPACE_NAME, - 'node_modules' - ); const packageImport = parsePackageImport(specifier); - const pathToNodeModule = path.join(nodeModules, packageImport.packageName); + const pathToNodeModule = path.join(nodeModulesPath, packageImport.packageName); - const isInternalNodePackage = !fs.existsSync(pathToNodeModule); - if (isInternalNodePackage) { - return defaultResolve(specifier, context, defaultResolve); + // If the module can be directly found in the `node_modules`, then we know it's + // a third-party package coming from NPM. In this case we properly respect ESM + // resolution by respecting the `exports`. + const npmModuleResult = fs.existsSync(pathToNodeModule) + ? resolvePackageWithExportsSupport(pathToNodeModule, packageImport) + : null; + if (npmModuleResult !== null) { + return npmModuleResult; } - const packageJson = JSON.parse( - fs.readFileSync(path.join(pathToNodeModule, 'package.json'), 'utf-8') - ); + // If the package does not exist on disk, then it may just be an invalid + // import, or the package is 1st-party one that is mapped within Bazel. + // We attempt to resolve it that way and return the path if there is a result. + const localMappingResult = tryResolveViaLocalMappings(specifier, packageImport); + if (localMappingResult !== null) { + return localMappingResult; + } - const localPackagePath = resolvePackageLocalFilepath(packageImport, packageJson); - const resolvedFilePath = path.join(pathToNodeModule, localPackagePath); - - return {url: pathToFileURL(resolvedFilePath).href}; + // Process built-in modules or unknown specifiers. + return nextResolve(specifier, context); } +/** Gets whether the specifier refers to a module. */ function isNodeOrNpmPackageImport(specifier) { return ( !specifier.startsWith('./') && @@ -58,6 +72,24 @@ function isNodeOrNpmPackageImport(specifier) { ); } +/** + * Attempts to resolve a specifier using the Bazel patched resolution, + * supporting first-party package mappings from `rules_nodejs`. + */ +function tryResolveViaLocalMappings(actualSpecifier) { + try { + const res = requireFn.resolve(actualSpecifier); + // Note: It may not resolve to a path if the specifier is a builtin + // module. In such cases we do not want to return it as result. + if (fs.existsSync(res)) { + return {url: pathToFileURL(res).href}; + } + } catch {} + + return null; +} + +/** Parses the given specifier into its package and subpath. */ function parsePackageImport(specifier) { const [, packageName, pathInPackage = ''] = /^((?:@[^/]+\/)?[^/]+)(?:\/(.+))?$/.exec(specifier) ?? []; @@ -67,10 +99,27 @@ function parsePackageImport(specifier) { return {packageName, pathInPackage, specifier}; } +/** Resolves an import to a module by respecting the `package.json` `exports`. */ +function resolvePackageWithExportsSupport(pathToNodeModule, packageImport) { + const packageJson = JSON.parse( + fs.readFileSync(path.join(pathToNodeModule, 'package.json'), 'utf8') + ); + const localPackagePath = resolvePackageLocalFilepath(packageImport, packageJson); + const resolvedFilePath = path.join(pathToNodeModule, localPackagePath); + + if (fs.existsSync(resolvedFilePath)) { + return {url: pathToFileURL(resolvedFilePath).href}; + } + return null; +} + +/** + * Resolves the remaining package-local portion of an import. Leverages + * the `package.json` `exports` field information for resolution. + */ function resolvePackageLocalFilepath(packageImport, packageJson) { if (packageJson.exports) { return resolveExports(packageJson, packageImport.specifier); } - return packageImport.pathInPackage || packageJson.module || packageJson.main || 'index.js'; } diff --git a/tools/esm-interop/index.bzl b/tools/esm-interop/index.bzl new file mode 100644 index 00000000000..9d551e6d0bf --- /dev/null +++ b/tools/esm-interop/index.bzl @@ -0,0 +1,23 @@ +"""ESM interop helpers.""" + +load("@npm//@angular/build-tooling/bazel:extract_js_module_output.bzl", "extract_js_module_output") +load("//tools/esm-interop:esm-node-module-loader.bzl", _enable_esm_node_module_loader = "enable_esm_node_module_loader") +load("//tools/esm-interop:esm-loaders.bzl", _install_esm_loaders = "install_esm_loaders") + +install_esm_loaders = _install_esm_loaders +enable_esm_node_module_loader = _enable_esm_node_module_loader + +def extract_esm_outputs(name, deps, testonly = False): + """"Extracts the ESM output variants from the given dependency.""" + + extract_js_module_output( + name = name, + deps = deps, + testonly = testonly, + tags = ["manual"], + provider = "JSEcmaScriptModuleInfo", + forward_linker_mappings = True, + include_external_npm_packages = True, + include_default_files = False, + include_declarations = False, + )