GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |
GeezerVison is showing github.com — build: classify the firebase symbols a version raise would newly wrap by armando-navarro · Pull Request #3759 · angular/angularfire Conversation
Collaborator
Checklist
Description
One of them does not merely gain nothing, it breaks. The auth persistence exports are classes, and Firebase constructs whichever one it is given with This PR settles the classification while the installed version is still 12.4.0, so the version raise stays a small change rather than a behavior change bundled with it. Nothing here alters generated output, because the generator builds its export list from the installed declarations, so an entry naming a symbol that version does not have is never consulted. What each entry decides:
Two supporting changes:
Two smaller items ride along:
TestingNo unit tests, because nothing here changes runtime behavior: the generated files are byte-identical before and after. The check that matters is that regenerating produces no diff, which is what shows the entries are inert against firebase 12.4.0. That is empty on this branch. Two further results:
If any of this reads wrong to you, particularly the four I am leaving unwrapped, say so and I will revisit before the version raise goes up. Refs #3756
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions…ything ngPostUpdate took a Tree and returned it unchanged. It was registered in migration.json, listed in the schematics tsconfig, loaded as a build entry point and bundled on every build, and running it was always a no-op. Its description claimed it printed results after ng-update, and it printed nothing. Leaving it in place costs more than the eight lines suggest. Anyone auditing what ng update does has to open the file to find out that the answer is nothing, and a dead entry point is one more thing the build has to keep loading. The entry was spelled ng-post-upgate, missing the d, so a search for the correct spelling did not find it. tools/build.ts refers to it as ['update', 'index'], assembled from parts, so a search for the path did not find that either. Removing it changes no behavior. Nothing referenced ngPostUpdate except the four places removed here, and the file exported nothing else. … the generator The build regenerates src/<module>/firebase.ts from the declarations of whatever firebase is installed, wrapping every lowercase-initial function. Raising the minimum version would wrap symbols nobody has classified, and one breaks: the auth persistence exports are classes that firebase constructs with `new`, and the wrapper replaces them with an ordinary function. These entries settle that first, and stay inert until the version rises. Do not wrap maximum, minimum, getTemplateGenerativeModel or makeMemoryCacheProvider: each is synchronous and returns a plain value, so wrapping only adds a warning to a call that cannot destabilize anything. Wrap the messaging subscriptions with blockUntilFirst off. With it on, subscribing holds a pending task until the callback fires, and these fire on a registration change that may never happen. The generator is now exported so `npm run generate` runs it without `ng build`, and it fails on an override key its entry point does not declare, which is otherwise ignored in silence. The guide said to call Firebase APIs inside an injection context and showed how, but never said what AngularFire does with one, so the warning read as a lint rule rather than a description of something lost. The new section says what AngularFire asks Angular for, what happens without it, and names the consequence: the call is never added to the PendingTasks register, which is what server-side rendering waits on before it serializes the page.
tyler-reitz
approved these changes
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Approving. Ran the checks rather than reading them: One question, not blocking. The misfiled Worth noting the guard is stronger than the PR says:
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Collaborator
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page.
armando-navarro
added a commit
to armando-navarro/angularfire
that referenced
this pull request
The generated src/<module>/firebase.ts files are written from the type declarations of whatever firebase is installed, so the set of functions AngularFire wraps is decided by the version the build resolves. That was 12.4.0 while npm latest reached 12.18.0, leaving newly added functions reaching callers through the star export with no zone integration and no pending-task registration. Raising the required version wraps them. Everything the override entries in angular#3759 classified as unwrapped stays out. getImagenModel loses its entry because firebase removed the symbol in 12.18.0, so the generator no longer sees it. The exemption list it sat in is empty now and stays as the place the next unclassified name goes. Fixes angular#3756
armando-navarro
added a commit
that referenced
this pull request
The generated src/<module>/firebase.ts files are written from the type declarations of the installed firebase, which decides which functions AngularFire wraps. That was 12.4.0 while npm latest reached 12.18.0, leaving everything firebase added in between to reach callers through `export *`, with no zone integration and no pending-task registration. Raising the required version wraps them. Everything the override entries in #3759 classified as unwrapped stays out. `getImagenModel` loses its entry because firebase removed the symbol in 12.18.0, and the exemption list it sat in is empty now and stays as the place the next unclassified name goes. Only `src/messaging/firebase.ts` changes among the generated files, gaining onRegistered, onUnregistered, register and unregister. `docs/messaging.md` was updated to use those calls. The Node send example keeps its token field, which Firebase says still accepts an installation ID during the migration, rather than the fid field it recommends, because firebase-admin 13.5.0 does not declare one. `exportsSeenPerOverrides` is keyed by the overrides object, and firestore and firestore/lite are handed the same firestoreOverrides, so a name listed for one silences the check for the other. Fixes #3756
This was referenced
armando-navarro
added a commit
that referenced
this pull request
ɵzoneWrap holds a pending task from the moment a wrapped call gets a callback until that callback first runs. For onLog, onTokenChanged, the Database child listeners (onChildAdded on an empty node, the other three until a later change), off, enableLogging, and the rxfire streams stateChanges and auditTrail, nothing guarantees that first run, so ApplicationRef.isStable can stay false for good and an SSR build's route extraction times out, as #3748 reported for beforeAuthStateChanged. They keep logLevel WARN, so only the pending task changes. isSupported in remote-config is the twelfth: #3613 put its blockUntilFirst: false entry in the rxfire block, which exports no isSupported, so it never applied, and #3759 removed the dead entry. Server renders no longer wait for onChildAdded's first batch (onValue, object and list still wait), and off(ref, type, cb) still detaches nothing, because the wrapper replaces the callback it is given. fromRef takes the event to listen for, and one blockUntilFirst value cannot fit both uses: value always fires on attach, while a child event may never fire. A hand-written fromRef in overrides.ts wraps rxfire's fromRef both ways and picks by event, so fromRef(ref, ListenEvent.value) holds a server render like object(ref) and child events do not. Fixes #3826
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ReviewersUh oh!
There was an error while loading. Please reload this page. AssigneesNo one assignedLabels
bump: patch
ProjectsDevelopmentSuccessfully merging this pull request may close these issues. Uh oh!
There was an error while loading. Please reload this page. Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later. | |
| You are visitor 01402503 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required. |