docs: fix broken code snippets in product guides - #3732
Conversation
Several product-guide snippets did not compile or parse as written, so a reader copying them hit errors before reaching Firebase. - Close the appConfig object literal with } instead of }) in the App Check, Auth, Database, Functions, Messaging, Remote Config, Storage, and Performance guides. - Import ApplicationConfig in every appConfig snippet so the type annotation resolves. - App Check: import getApp, which the snippet calls. - Firestore: correct "export Interface" to "export interface" (twice), import Observable from rxjs, and import CollectionReference and DocumentReference. - Auth and Database: replace the NgModule emulator and multi-instance examples with the standalone appConfig form the rest of the guides teach, and inline the Firebase config instead of reading a no-longer-generated environment file. - Messaging: repair the FcmService example (declare message$, move its assignment into the constructor, make deleteToken async) and bump the service-worker CDN imports from 9.22.0 to 12.4.0 to match the firebase dependency.
tyler-reitz
left a comment
There was a problem hiding this comment.
Nice cleanup — these are the samples people copy first, so having them parse matters.
I pulled the branch and ran the repaired FcmService through tsc; the only remaining diagnostics are unresolved module specifiers from --noResolve, so the brace structure now parses where it previously did not. The 9.22.0 to 12.4.0 service-worker CDN bump matches the firebase: ^12.4.0 dependency. Returning onMessage's unsubscribe from the new Observable(...) subscriber function is also correct RxJS teardown, so that sample no longer leaks its listener.
Two nits, neither blocking:
message$: Observable<unknown>loses the payload type a reader would want —Observable<MessagePayload>would be more useful as a starting point.- Inside
async deleteToken(), the call to the importeddeleteToken(this.msg)is correct but reads like recursion at a glance. An import alias or a one-line comment would save the double-take.
Also worth noting for merge order: this and #3734 both touch docs/app-check.md and docs/auth.md with adjacent hunks. They don't duplicate each other, but whichever lands second should be rebased and re-skimmed rather than merged on a stale green.
The FcmService sample declared message$ as Observable<unknown>, which hides the payload shape a reader wants, and its deleteToken method calls the imported deleteToken, which reads as recursion at a glance. message$ is now Observable<MessagePayload>. The constructor needs the explicit type argument on new Observable as well: without it the call infers unknown and the assignment fails with TS2345. Verified by compiling the sample under strict against the built @angular/fire/messaging types, which fails if the type argument is dropped.
|
Both taken, thanks. Pushed as a follow-up commit. The two changes
One wrinkle on the typingChanging only the field annotation does not compile. The |
Description
Fixes the broken copy-paste code samples in the modular product guides. Each change makes the sample compile or parse as written, so a developer can copy it directly.
appConfigobject literal with}(was})) in the App Check, Auth, Database, Functions, Messaging, Remote Config, Storage, and Performance guides.ApplicationConfigin everyappConfigsample so the type annotation resolves.getApp, which the sample calls.export Interfacetoexport interface(twice), importObservablefrom rxjs, and importCollectionReferenceandDocumentReference.@NgModuleemulator and multiple-instance samples with the standaloneappConfigform the rest of the guides teach, and inline the Firebase config instead of readingenvironment.firebase, which the currentng addsetup does not generate.FcmServiceexample (declaremessage$, move its assignment into the constructor, makedeleteTokenasync) and bump the service-worker CDN imports from 9.22.0 to 12.4.0 to match the firebase dependency.Documentation only, no source or API changes.
Checklist
yarn install,yarn testrun successfully?: not applicable, no code changedFixes #3731
Refs #3369