Skip to content

console build for rtdb #3709

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 5 commits into from
Sep 17, 2020
Merged

console build for rtdb #3709

merged 5 commits into from
Sep 17, 2020

Conversation

Feiyang1
Copy link
Member

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Aug 31, 2020

💥 No Changeset

Latest commit: 42c0025

Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂

If these changes should be published to npm, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 31, 2020

Size Analysis Report

Affected Products

No changes between base commit (ff66d8c) and head commit (96420d8).

Test Logs

Comment on lines 94 to 109
export function initStandalone<T>({
app,
url,
version,
customAuthImpl,
namespace
}: {
app: FirebaseApp;
url: string;
version: string;
customAuthImpl: FirebaseAuthInternal;
namespace: T;
}): {
instance: types.Database;
namespace: T;
} {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not married to the original way this was implemented, but I don't see much benefit of splitting this code across two different functions that are both named the same. Reducing this back down to one method would also remove the options object, which marginally improves readability but adds some duplication to ensure type safety. What do you think of moving this code back?

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved it here to make it available via firebase.database.INTERNAL.initStandalone, so console can consume the normal cdn script without needing a special build target.

https://github.com/firebase/firebase-js-sdk/blob/master/packages/database/index.node.ts#L139

@google-oss-bot
Copy link
Contributor

Binary Size Report

Affected SDKs

  • @firebase/database

    Type Base (ff66d8c) Head (96420d8) Diff
    browser 270 kB 270 kB +567 B (+0.2%)
    esm2017 236 kB 236 kB +509 B (+0.2%)
    main 270 kB 271 kB +344 B (+0.1%)
    module 268 kB 269 kB +565 B (+0.2%)
  • firebase

    Type Base (ff66d8c) Head (96420d8) Diff
    firebase-database.js 187 kB 190 kB +3.31 kB (+1.8%)
    firebase.js 830 kB 830 kB +344 B (+0.0%)

Test Logs

@Feiyang1 Feiyang1 merged commit d41152a into master Sep 17, 2020
@Feiyang1 Feiyang1 deleted the fei-rtdb-console branch September 17, 2020 18:12
@firebase firebase locked and limited conversation to collaborators Oct 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants