Skip to content

Fix targetType re-instantiation with constructor arguments when non primitive type is passed #1826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

tbhaxor
Copy link

@tbhaxor tbhaxor commented Jun 14, 2025

Description

Some classes accept arguments via their constructors and include internal validations. One such example is the Timestamp class defined in firebase-admin/firestore, which performs validation during initialisation.

This PR extracts the constructor arguments from the class source string, maps the provided values to the corresponding constructor parameters (assuming the parameters are always assigned to class properties), and uses them to reinitialise the class instance.

Tip

Ideally, value parameter sent to transform() function should be passed directly in such cases. It's unclear why this approach wasn't taken. But since original authors used re-instantiate approach, I am keeping the same.

Checklist

  • the pull request title describes what this PR does (not a vague title like Update index.md)
  • the pull request targets the default branch of the repository (develop)
  • the code follows the established code style of the repository
    • npm run prettier:check passes
    • npm run lint:check passes
  • tests are added for the changes I made (if any source code was modified)
  • documentation added or updated
  • I have run the project locally and verified that there are no errors

Fixes

Fixes #1825

Note

This is an internal fix, so documentation updates are not necessary. Let me know if you think it should be documented, and where exactly it should go.

tbhaxor added 4 commits June 15, 2025 03:04
It resolves warning from jest during `npm test`.
Specs for get-global file not being used. Renamed to spect
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

question: Transforming firestore Timestamp works with firebase/firestore, but not firebase-admin/firestore
1 participant