diff --git a/docs/zones.md b/docs/zones.md index 24a8a088b..99aa0de58 100644 --- a/docs/zones.md +++ b/docs/zones.md @@ -49,6 +49,19 @@ readonly refreshed$ = this.refresh$.pipe(switchMap(() => this.items$)); Reach for `runInInjectionContext` only when the call genuinely must run inside the callback, as the per-user query above does (it needs the signed-in user's `uid`). +### Why the injection context is required + +AngularFire cannot create an injection context, only borrow the one you are already in. + +When you call a wrapped API, the first thing AngularFire does is ask Angular for three things with `inject()`: its own scheduler service, Angular's `PendingTasks` register, and an `EnvironmentInjector`. `inject()` only works inside an injection context, so outside of one the first of them throws and the rest are never reached. AngularFire catches that, warns while in dev-mode as described under Logging below, and calls the Firebase API directly with nothing added. + +That division is worth knowing, because it explains what you are and are not responsible for: + +* **You** supply the context at the call site, from a field initializer, a constructor, or an explicit `runInInjectionContext`. +* **AngularFire** re-enters the environment injector inside a callback you hand to the call itself, which is why you never wrap an `onSnapshot` handler yourself. That reaches anything provided at the application level, but not providers declared on a component, and it does not extend to subscribers of a returned Observable, which is what the `switchMap` and `runInInjectionContext` example above is for. + +What you lose depends on the call. A call that does asynchronous work, such as `onSnapshot`, `getDoc` or `collectionData`, is normally added to Angular's `PendingTasks` register, which is what server-side rendering waits on before it serializes the page. Called outside a context, it never is. A call that returns immediately, such as `getFirestore`, was never registered anyway, so it loses only the zone handling. + ## Logging You may see a log warning, `Calling Firebase APIs outside of an Injection context may destabilize your application leading to subtle change-detection and hydration bugs. Find more at https://github.com/angular/angularfire/blob/main/docs/zones.md` when developing your application. diff --git a/package.json b/package.json index cffe4c530..2dff2aad9 100644 --- a/package.json +++ b/package.json @@ -23,6 +23,7 @@ "test:build": "bash ./test/ng-build/build.sh", "test:all": "npm run test:node-esm && npm run test:chrome-headless && npm run test:typings && npm run test:build", "build": "rimraf dist && tspc -p tsconfig.build.json && node --trace-warnings ./tools/build.js && npm pack ./dist/packages-dist", + "generate": "tspc -p tsconfig.build.json && node -e \"require('./tools/build.js').zoneWrapExports().catch(err => { console.error(err); process.exit(1); })\"", "buildd": "tsc -p tsconfig.build.json && node --trace-warnings ./tools/build.js && npm pack ./dist/packages-dist", "build:jasmine": "npx tsc -p tsconfig.jasmine.json --module es2015 && cp ./dist/out-tsc/jasmine/tools/jasmine.js ./dist/out-tsc/jasmine/tools/jasmine.mjs && npx tsc -p tsconfig.jasmine.json && cp ./dist/packages-dist/schematics/versions.json ./dist/out-tsc/jasmine/src/schematics", "changelog": "conventional-changelog -p angular -i CHANGELOG.md -s -r 1" diff --git a/src/schematics/migration.json b/src/schematics/migration.json index 7c040e6a1..1ac41eca4 100644 --- a/src/schematics/migration.json +++ b/src/schematics/migration.json @@ -10,11 +10,6 @@ "version": "21.0.0", "description": "Align the workspace's firebase dependency with the range @angular/fire 21 requires, and rewrite Vertex AI imports to Firebase AI Logic (getVertexAI callers keep the Vertex AI backend)", "factory": "./update/v21#ngUpdate" - }, - "ng-post-upgate": { - "description": "Print out results after ng-update", - "factory": "./update#ngPostUpdate", - "private": true } } } \ No newline at end of file diff --git a/src/schematics/tsconfig.json b/src/schematics/tsconfig.json index bc0b86afd..fe767ea40 100644 --- a/src/schematics/tsconfig.json +++ b/src/schematics/tsconfig.json @@ -22,7 +22,6 @@ "outDir": "../../dist/packages-dist/schematics" }, "files": [ - "update/index.ts", "deploy/actions.ts", "deploy/builder.ts", "add/index.ts", diff --git a/src/schematics/update/index.ts b/src/schematics/update/index.ts deleted file mode 100644 index 7b57b5ab4..000000000 --- a/src/schematics/update/index.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { Rule, SchematicContext, Tree } from '@angular-devkit/schematics'; - -export const ngPostUpdate = (): Rule => ( - host: Tree, - _context: SchematicContext -) => { - return host; -}; diff --git a/tools/build.ts b/tools/build.ts index a2a704bcf..b5bf8c003 100644 --- a/tools/build.ts +++ b/tools/build.ts @@ -37,9 +37,13 @@ const firestoreOverrides = { increment: { logLevel: LogLevel.VERBOSE }, limit: { logLevel: LogLevel.VERBOSE }, limitToLast: { logLevel: LogLevel.VERBOSE }, + // maximum and minimum take no callback and return no promise; wrapping would not benefit them. + maximum: null, memoryEagerGarbageCollector: null, memoryLocalCache: null, memoryLruGarbageCollector: null, + // See maximum above. + minimum: null, namedQuery: { logLevel: LogLevel.VERBOSE }, or: { logLevel: LogLevel.VERBOSE }, orderBy: { logLevel: LogLevel.VERBOSE }, @@ -61,14 +65,38 @@ const firestoreOverrides = { writeBatch: { logLevel: LogLevel.VERBOSE }, }; +/* Override keys firebase's type declarations may not carry, which the generator would otherwise + * read as typos. A name lands here because a later release adds it, firebase has removed it, or + * firebase keeps it out of its public types on purpose (browserCookiePersistence). Listing a + * declared name is harmless: getImagenModel is here for the version this build is heading to. + * Keyed by entry point, so a name allowed for one module is not allowed for the rest. + * At a version bump, drop a name from here but KEEP its override entry. */ +const overridesFirebaseDoesNotDeclare: Record = { + 'firebase/ai': ['getImagenModel', 'getTemplateGenerativeModel'], + 'firebase/auth': ['browserCookiePersistence'], + 'firebase/data-connect': ['makeMemoryCacheProvider'], + 'firebase/firestore': ['maximum', 'minimum'], + 'firebase/messaging': ['onRegistered', 'onUnregistered', 'unregister'], +}; + +type Overrides = Record; + function zoneWrapExports() { + /* One overrides object can be applied to several entry points, so the keys it may legally name + * are the union of their export lists. firestoreOverrides covers both firestore and + * firestore/lite, and only firestore has persistentLocalCache and the rest. */ + const exportsSeenPerOverrides = new Map>(); const reexport = async ( module: string, name: string, path: string, exports: string[], - overrides: Record = {} + overrides: Overrides = {} ) => { + const seen = exportsSeenPerOverrides.get(overrides) ?? new Set(); + exports.forEach(exportName => seen.add(exportName)); + (overridesFirebaseDoesNotDeclare[path] ?? []).forEach(undeclared => seen.add(undeclared)); + exportsSeenPerOverrides.set(overrides, seen); const imported = await import(path); const toBeExported: [string, string, boolean][] = exports.sort(). filter(it => !it.startsWith('_') && overrides[it] !== null && overrides[it]?.override !== true). @@ -113,12 +141,23 @@ ${exportedZoneWrappedFns} `; await writeFile(filePath, fileOutput); }; + const failOnUnrecognizedOverrides = () => { + const unrecognized = [...exportsSeenPerOverrides].flatMap(([overrides, seen]) => + Object.keys(overrides).filter(key => !seen.has(key) && overrides[key]?.override !== true)); + if (unrecognized.length) { + throw new Error( + `Override keys their entry point does not declare: ${unrecognized.join(', ')}. ` + + 'Overrides are matched by name, so this one is silently ignored and the symbol keeps the ' + + 'default. Fix the spelling, move it to the right block, or add it to ' + + 'overridesFirebaseDoesNotDeclare.' + ); + } + }; return Promise.all([ reexport('ai', 'firebase', 'firebase/ai', tsKeys(), { - // Removed in @firebase/ai 2.15.0 (firebase 12.18.0), which the ^12.4.0 range - // resolves for fresh installs. A named import here would make consumer builds - // fail on that version, so only re-export it through the star export, which - // tracks whichever firebase is installed. + // Unwrapped via the star export: no callback, returns the model object directly. + getTemplateGenerativeModel: null, + // Removed in @firebase/ai 2.15.0 (firebase 12.18.0). getImagenModel: null, }), reexport('analytics', 'firebase', 'firebase/analytics', tsKeys(), { @@ -138,7 +177,11 @@ ${exportedZoneWrappedFns} reexport('auth', 'rxfire', 'rxfire/auth', tsKeys()), reexport('auth', 'firebase', 'firebase/auth', tsKeys(), { debugErrorMap: null, + /* These 5 persistence entries MUST stay unwrapped. Though their type declarations + * disagree, they are classes, and wrapping would replace them with ordinary functions + * that throw when used with `new`. */ inMemoryPersistence: null, + browserCookiePersistence: null, browserLocalPersistence: null, browserSessionPersistence: null, indexedDBLocalPersistence: null, @@ -209,6 +252,8 @@ ${exportedZoneWrappedFns} update: { logLevel: LogLevel.VERBOSE }, }), reexport('data-connect', 'firebase', 'firebase/data-connect', tsKeys(), { + // Unwrapped via the star export: no callback, returns a plain settings value. + makeMemoryCacheProvider: null, mutationRef: { logLevel: LogLevel.VERBOSE }, queryRef: { logLevel: LogLevel.VERBOSE }, toQueryRef: { logLevel: LogLevel.VERBOSE }, @@ -225,10 +270,14 @@ ${exportedZoneWrappedFns} reexport('messaging', 'firebase', 'firebase/messaging', tsKeys(), { isSupported: { blockUntilFirst: false }, onMessage: { blockUntilFirst: false }, + // `blockUntilFirst: false` otherwise `ApplicationRef.isStable` may never become `true`. + onRegistered: { blockUntilFirst: false }, + onUnregistered: { blockUntilFirst: false }, deleteToken: { logLevel: LogLevel.VERBOSE }, + // Quiets the per-call log line, matching deleteToken above. + unregister: { logLevel: LogLevel.VERBOSE }, }), reexport('remote-config', 'rxfire', 'rxfire/remote-config', tsKeys(), { - isSupported: { blockUntilFirst: false }, getValue: { exportName: 'getValueChanges' }, getString: { exportName: 'getStringChanges' }, getNumber: { exportName: 'getNumberChanges' }, @@ -259,7 +308,7 @@ ${exportedZoneWrappedFns} collection: { exportName: 'collectionSnapshots' }, }), reexport('firestore/lite', 'firebase', 'firebase/firestore/lite', tsKeys(), firestoreOverrides), - ]); + ]).then(failOnUnrecognizedOverrides); } const src = (...args: string[]) => join(process.cwd(), 'src', ...args); @@ -331,7 +380,6 @@ function spawnPromise(command: string, args: string[]) { // Path segments of each schematic entry point, relative to `schematics/` and without the file // extension: esbuild compiles the `.ts` and loadCompiledSchematics requires the emitted `.js`. const schematicEntryPoints = [ - ['update', 'index'], ['deploy', 'actions'], ['deploy', 'builder'], ['add', 'index'], @@ -409,7 +457,12 @@ async function buildLibrary() { ]); } -buildLibrary().catch(err => { - console.error(err); - process.exit(1); -}) +// Exported so `npm run generate` can run the code generation on its own. +export { zoneWrapExports }; + +if (require.main === module) { + buildLibrary().catch(err => { + console.error(err); + process.exit(1); + }); +}