Skip to content

Commit c4c4ad1

Browse files
fix(schematics): restore ng deploy under the CommonJS schematics bundle (#3729)
* fix(schematics): restore ng deploy under the CommonJS schematics bundle `ng deploy` threw at module load in 21.0.0-rc.0, before any user code ran: TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string or an instance of URL. Received undefined at fileURLToPath (node:internal/url) The schematics are bundled by esbuild with `format: "cjs"`, and esbuild rewrites `import.meta` to an empty object in CommonJS output. The shipped bundle therefore read `undefined.url`, so both `deploy/actions.js` and `deploy/builder.js` failed to load. Every other shipped entry point (ng add, both ng update migrations, the setup schematic) was unaffected. The shim was introduced when `versions.json` moved from a compile-time import to a runtime read. That move fixed a real bug of its own: because esbuild bundles before the build copies and rewrites `versions.json`, the compile-time import inlined the unreplaced `0.0.0` placeholders, and 20.0.1 generates a Cloud Functions manifest pinning `0.0.0` that cannot install. So the runtime read has to stay. `typeof` on an undeclared identifier is the one form that does not throw under ESM, so a single expression works under both loaders, and the CommonJS branch comes first because `import.meta` is the substituted empty object there. The alternatives were built and run, not assumed: - plain `__dirname` breaks `npm run test:node-esm`, which genuinely loads the compiled specs as ESM - `require('../versions.json')` reintroduces the `0.0.0` bug above - an esbuild define/banner works today but fails with "require is not defined in ES module scope" the moment `format: "esm"` is enabled, which tools/build.ts already has staged in a comment Verified against the built package: all seven shipped entry points now load via both `require()` and `await import()`, the builder exposes the Architect builder symbols, and the runtime `versions.json` read resolves correctly. `ng lint` also drops its only warning, which sat on the replaced line. This is v21-only. v20 has no `import.meta` shim and must not take this change. * build: load every compiled schematic before publishing The load failure fixed in the previous commit reached a published release because nothing in the build or the test suite ever loads what actually ships. The jasmine suite runs against the TypeScript output, which is a different module format from the CommonJS bundle in the package, so a bundle can be completely unloadable while every test passes. Requiring each compiled entry point at the end of the schematics build closes that gap. Reverting the previous commit now fails the build with the real error: Compiled schematics failed to load: deploy/actions.js: TypeError [ERR_INVALID_ARG_TYPE] ... deploy/builder.js: TypeError [ERR_INVALID_ARG_TYPE] ... It catches the whole class, not just this instance: an unresolvable import, a bad top-level require, or anything else that throws at module load. * build: derive schematic entry points from a single list compileSchematics and loadCompiledSchematics each carried their own hardcoded copy of the seven entry points. An entry point added to the esbuild list alone would compile but never be load-checked, which is the exact failure the load check exists to catch. Both now map one schematicEntryPoints array, to .ts for esbuild and to .js for the require check. Emitted paths and failure strings are unchanged: a build with a deliberate top-level throw added to deploy/actions.ts still fails, and still names deploy/actions.js and deploy/builder.js in the same format.
1 parent a99f09b commit c4c4ad1

2 files changed

Lines changed: 36 additions & 12 deletions

File tree

src/schematics/deploy/actions.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@ import * as winston from 'winston';
1313
import { BuildTarget, CloudRunOptions, DeployBuilderSchema, FSHost, FirebaseTools } from '../interfaces';
1414
import { DEFAULT_FUNCTION_NAME, defaultFunction, defaultPackage, dockerfile, functionGen2 } from './functions-templates.js';
1515

16-
// @ts-ignore
17-
const __dirname = dirname(fileURLToPath(import.meta.url));
16+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
17+
// @ts-ignore `import.meta` is rejected by the --module es2015 pass of `npm run build:jasmine`.
18+
const moduleDirectory = typeof __dirname === 'string' ? __dirname : dirname(fileURLToPath(import.meta.url));
1819

1920
const { copySync, removeSync, readJsonSync } = fsExtra;
2021

@@ -128,7 +129,7 @@ const findPackageVersion = (packageManager: string, name: string) => {
128129
const getPackageJson = (context: BuilderContext, workspaceRoot: string, options: DeployBuilderOptions, main?: string) => {
129130
const dependencies: Record<string, string> = {};
130131
const devDependencies: Record<string, string> = {};
131-
const { firebaseFunctionsDependencies } = readJsonSync(join(__dirname, '..', 'versions.json'));
132+
const { firebaseFunctionsDependencies } = readJsonSync(join(moduleDirectory, '..', 'versions.json'));
132133
if (options.ssr !== 'cloud-run') {
133134
Object.keys(firebaseFunctionsDependencies).forEach(name => {
134135
const { version, dev } = firebaseFunctionsDependencies[name];

tools/build.ts

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -313,17 +313,21 @@ function spawnPromise(command: string, args: string[]) {
313313
.on('error', reject));
314314
}
315315

316+
// Path segments of each schematic entry point, relative to `schematics/` and without the file
317+
// extension: esbuild compiles the `.ts` and loadCompiledSchematics requires the emitted `.js`.
318+
const schematicEntryPoints = [
319+
['update', 'index'],
320+
['deploy', 'actions'],
321+
['deploy', 'builder'],
322+
['add', 'index'],
323+
['setup', 'index'],
324+
['update', 'v7', 'index'],
325+
['update', 'v21', 'index'],
326+
];
327+
316328
async function compileSchematics() {
317329
await esbuild.build({
318-
entryPoints: [
319-
src('schematics', "update", "index.ts"),
320-
src('schematics', "deploy", "actions.ts"),
321-
src('schematics', "deploy", "builder.ts"),
322-
src('schematics', "add", "index.ts"),
323-
src('schematics', "setup", "index.ts"),
324-
src('schematics', "update", "v7", "index.ts"),
325-
src('schematics', "update", "v21", "index.ts"),
326-
],
330+
entryPoints: schematicEntryPoints.map(segments => `${src('schematics', ...segments)}.ts`),
327331
format: "cjs",
328332
// turns out schematics don't support ESM, need to use webpack or shim these
329333
// format: "esm",
@@ -357,6 +361,25 @@ async function compileSchematics() {
357361
copy(src('schematics', 'setup', 'schema.json'), dest('schematics', 'setup', 'schema.json')),
358362
]);
359363
await replaceSchematicVersions();
364+
await loadCompiledSchematics();
365+
}
366+
367+
/**
368+
* Loads every compiled schematic entry point, so a bundle that cannot even be required fails the
369+
* build instead of shipping.
370+
*/
371+
async function loadCompiledSchematics() {
372+
const failures: string[] = [];
373+
for (const segments of schematicEntryPoints) {
374+
try {
375+
require(`${dest('schematics', ...segments)}.js`);
376+
} catch (error) {
377+
failures.push(` ${join(...segments)}.js: ${error}`);
378+
}
379+
}
380+
if (failures.length) {
381+
throw new Error(`Compiled schematics failed to load:\n${failures.join('\n')}`);
382+
}
360383
}
361384

362385
async function buildLibrary() {

0 commit comments

Comments
 (0)