Skip to content

Fix Data Connect Types #8898

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

Merged
merged 4 commits into from
Apr 4, 2025
Merged

Fix Data Connect Types #8898

merged 4 commits into from
Apr 4, 2025

Conversation

maneesht
Copy link
Contributor

@maneesht maneesht commented Apr 4, 2025

DataConnectOperationError extends DataConnectError, both of which have tags @hideconstructor. This means that in public builds, our output marks the constructors as private. However, this causes an error to be thrown: cannot extend private constructor, because DataConnectOperationError extends DataConnectError, which has a private constructor. The reason why this wasn't caught during integration tests is because to integration tests, the constructors aren't private, but for public builds (that are used by external devs), the constructors are private. So the only way we could've caught this would've been to use the published version (or packaged version) of the SDK.

TL;DR: Fix error where if we exported DataConnectError, then when using the types, the user would get a cannot extend private constructor due to the fact that DataConnectOperationError would extend DataConnectError.

The fix for this is to remove the @hideconstructor annotation. The right fix would have been to not export DataConnectError at all, but if we do that, it would result in a breaking change for those who use DataConnectError. (Though, the argument can be made that no one is even able to use that type because the types are broken).

@maneesht maneesht requested review from aashishpatil-g and a team as code owners April 4, 2025 17:33
Copy link

changeset-bot bot commented Apr 4, 2025

🦋 Changeset detected

Latest commit: 3339ff3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@firebase/data-connect Patch
firebase Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Apr 4, 2025

Vertex AI Mock Responses Check ⚠️

A newer major version of the mock responses for Vertex AI unit tests is available. update_vertexai_responses.sh should be updated to clone the latest version of the responses: v8.0

@maneesht maneesht requested review from a team as code owners April 4, 2025 17:33
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 4, 2025

Size Report 1

Affected Products

  • @firebase/data-connect

    TypeBase (88a8055)Merge (0c901ca)Diff
    browser21.4 kB21.4 kB+5 B (+0.0%)
    main23.6 kB23.7 kB+51 B (+0.2%)
    module21.4 kB21.4 kB+5 B (+0.0%)
  • firebase

    TypeBase (88a8055)Merge (0c901ca)Diff
    firebase-data-connect.js17.6 kB17.9 kB+358 B (+2.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/mYcoK4PRvh.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 4, 2025

Size Analysis Report 1

Affected Products

  • @firebase/data-connect

    • Code

      Size

      TypeBase (88a8055)Merge (0c901ca)Diff
      size?12.6 kB? (?)
      size-with-ext-deps?30.8 kB? (?)

      Dependency

      TypeBase (88a8055)Merge (0c901ca)Diff
      functions?

      15 dependencies

      addToken
      areTransportOptionsEqual
      compareDates
      dcFetch
      getGoogApiClientValue
      getMessage
      getRefSerializer
      logDebug
      logError
      parseOptions
      registerDataConnect
      setEncoder
      setIfNotExists
      setSDKVersion
      urlBuilder

      ?
      classes?

      AppCheckTokenProvider
      DataConnect
      DataConnectError
      DataConnectOperationError
      FirebaseAuthProvider
      MutationManager
      QueryManager
      RESTTransport

      ?
      variables?

      12 dependencies

      CallerSdkTypeEnum
      Code
      FIREBASE_DATA_CONNECT_EMULATOR_HOST_VAR
      QUERY_STR
      SDK_VERSION
      SOURCE_CACHE
      SOURCE_SERVER
      connectFetch
      encoderImpl
      logger
      name
      version

      ?
      enums??

      External Dependency

      ModuleBase (88a8055)Merge (0c901ca)Diff
      @firebase/app?

      SDK_VERSION
      _isFirebaseServerApp
      _registerComponent
      _removeServiceInstance
      registerVersion

      ?
      @firebase/component?

      Component

      ?
      @firebase/logger?

      Logger

      ?
      @firebase/util?

      FirebaseError

      ?

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/epYXRtug1K.html

@maneesht maneesht merged commit 1df3d26 into main Apr 4, 2025
37 checks passed
@maneesht maneesht deleted the mtewani/fix-private-error branch April 4, 2025 18:55
hsubox76 pushed a commit that referenced this pull request Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants