Search code, repositories, users, issues, pull requests...

Copy link

Contributor

@markgoho markgoho commented Sep 10, 2025 •

edited

Loading

Checklist

Issue number for this PR: update sample app to Angular 20 #3669 (required)
Docs included?: no significant changes
Test units included?: yes, and they pass now 🤓
In a clean directory, yarn install, yarn test run successfully? yes, for the sample directory

Description

Updating sample app to Angular 20

Author's Notes

installing angular/fire v20 was fun, I had to initially uninstall it from the sample app, update to Angular 20, then build v20 from source, then add it back in
many of the changes are lint-related
Karma tests are passing with the inclusion of a test.ts file -- it kept complaining about needing Zone.js even though this is a zoneless app, since there's only 3 tests, we may want to explore Vitest as an alternative
sadly, I did need to pass --no-verify when committing this, there are ~350 lint errors in the repo and I didn't want to include those fixes in this change
both client and server apps run, no errors appear locally, and most functionality that I could test continues to work
many, many updates needed to this app to bring it in line with newer patterns in Angular that have evolved since Signals came out, but we can tackle those in future PRs

markgoho added 3 commits

September 7, 2025 23:57


 install deps

c7b5bd1


 update angular

4bba3a9


 update sample app to angular 20

f1bed58

markgoho

markgoho commented

sample/src/test.ts

Copy link

Contributor Author

@markgoho markgoho Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new file courtesy of grok-code-fast-1, we're doing something similar in other sample apps in the project, but if there's something here we don't need, let's call that out

markgoho

markgoho commented

sample/src/server.ts

if (isMainModule(import.meta.url)) {

// eslint-disable-next-line @typescript-eslint/dot-notation

const port = process.env['PORT'] || 4000;

Copy link

Contributor Author

@markgoho markgoho Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both process.env.PORT (trying to access a property on an index type) and process.env['PORT'] (please use dot notation) have some sort of error associated with it, I went with this option

we may want to take a look at lint rules in the future

markgoho

markgoho commented

sample/src/app/app.component.ts

Comment on lines +28 to +35

@defer (hydrate on idle) { <app-database /> } @placeholder { Database!

… } @defer (hydrate on idle) { <app-firestore /> } @placeholder {

Firestore! … } @defer (hydrate on idle) { <app-functions /> }

@placeholder { Functions! … } @defer (hydrate on idle) {

<app-messaging /> } @placeholder { Messaging! … } @defer (hydrate on

idle) { <app-remote-config /> } @placeholder { Remote Config! … }

@defer (hydrate never) { <app-storage /> } @placeholder { Storage! …

} @defer (hydrate on idle) { <app-upboats /> } @placeholder { … }

Copy link

Contributor Author

@markgoho markgoho Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend we start using prettier for formatting (something to think about)

markgoho

markgoho commented

sample/package.json

"scripts": {

"ng": "ng",

"start": "firebase emulators:exec --import seed \"ng serve\"",

"start": "npx --yes firebase-tools@latest emulators:exec --import seed \"ng serve\"",

Copy link

Contributor Author

@markgoho markgoho Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe someone had firebase-tools installed globally, but I don't and I'd prefer to follow the pattern I see in the root package.json of installing this on demand

alternatively, we could install the package in the devDependencies and keep it up to date

Copy link

Contributor Author

markgoho commented Sep 25, 2025

@jamesdaniels any feedback for this PR?