Skip to content
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

Add Provider.initialize() #4595

Merged
merged 7 commits into from
Mar 12, 2021
Merged

Add Provider.initialize() #4595

merged 7 commits into from
Mar 12, 2021

Conversation

Feiyang1
Copy link
Member

@Feiyang1 Feiyang1 commented Mar 8, 2021

Currently component factory doesn't take an options parameter. This leads to a 2 steps initialization pattern in initializeAuth(), initializeFirestore(), initializePerformance() and initializeAnalytics():

Step 1: The component framework creates a service instance using default options
Step 2: Set user supplied options on the service instance returned from Step 1.

This PR removes the need of the second step by allowing you to pass an options to the component framework
using the new Provider.initialize() method.

2 steps initialization is also problematic for certain use cases. See this doc for more details.

Changes

Component factory can take an options parameter now:

// Now it accepts an options parameter
Type InstanceFactory<T extends Name> = (container: ComponentContainer, options: InstanceFactoryOptions) => NameServiceMapping[T];

interface InstanceFactoryOptions {
    instanceIdentifier?: string;
    options?: Record<string, unknown>;
}

Use initialize() to initialize with options. Taking initializePerformance() as an example:

export function initializePerformance(
 app: FirebaseApp,
 settings?: PerformanceSettings
): FirebasePerformance {
 const provider = _getProvider(app, 'performance-exp');

 if (provider.isInitialized()) {
   throw ERROR_FACTORY.create(ErrorCode.ALREADY_INITIALIZED);
 }
 
 // initialize a performance service instance with custom settings
 const perfInstance = provider.initialize(settings) as PerformanceController;
 
 // 2 steps process previously
 // const perfInstance = provider.getImmediate() as PerformanceController;
 // perfInstance._init(settings);

 return perfInstance;
}

@changeset-bot
Copy link

changeset-bot bot commented Mar 8, 2021

🦋 Changeset detected

Latest commit: 164a4e3

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

This PR includes changesets to release 34 packages
Name Type
@firebase/component Minor
@firebase/database Patch
@firebase/firestore Patch
@firebase/functions Patch
@firebase/remote-config Patch
@firebase/storage Patch
@firebase/analytics Patch
@firebase/app Patch
@firebase/installations Patch
@firebase/messaging Patch
@firebase/performance Patch
firebase Patch
@firebase/rules-unit-testing 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

@Feiyang1 Feiyang1 changed the title Add initialize() to Provider Add Provider.initialize() Mar 9, 2021
Copy link
Member

@yuchenshi yuchenshi left a comment

Choose a reason for hiding this comment

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

Approval for auth

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 9, 2021

Binary Size Report

Affected SDKs

  • @firebase/app

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 10.8 kB 10.8 kB -1 B (-0.0%)
    esm2017 9.57 kB 9.57 kB -1 B (-0.0%)
    lite 8.88 kB 8.88 kB -1 B (-0.0%)
    lite-esm2017 7.87 kB 7.87 kB -1 B (-0.0%)
    main 9.93 kB 9.93 kB -1 B (-0.0%)
    module 10.8 kB 10.8 kB -1 B (-0.0%)
    react-native 9.64 kB 9.64 kB -1 B (-0.0%)
  • @firebase/component

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 5.37 kB 6.18 kB +812 B (+15.1%)
    esm2017 4.08 kB 4.77 kB +695 B (+17.1%)
    main 5.69 kB 6.50 kB +812 B (+14.3%)
    module 5.37 kB 6.18 kB +812 B (+15.1%)
  • @firebase/database

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 275 kB 275 kB +22 B (+0.0%)
    esm2017 248 kB 248 kB +177 B (+0.1%)
    main 277 kB 277 kB +22 B (+0.0%)
    module 275 kB 275 kB +22 B (+0.0%)
  • @firebase/database-exp

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 272 kB 272 kB +22 B (+0.0%)
    esm2017 245 kB 245 kB +177 B (+0.1%)
    main 273 kB 273 kB +22 B (+0.0%)
    module 272 kB 272 kB +22 B (+0.0%)
  • @firebase/firestore

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 259 kB 259 kB -5 B (-0.0%)
    esm2017 202 kB 202 kB -5 B (-0.0%)
    main 530 kB 530 kB -1 B (-0.0%)
    module 259 kB 259 kB -5 B (-0.0%)
    react-native 202 kB 202 kB -5 B (-0.0%)
  • @firebase/firestore-exp

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 199 kB 199 kB +20 B (+0.0%)
    main 505 kB 505 kB +42 B (+0.0%)
    module 199 kB 199 kB +20 B (+0.0%)
    react-native 200 kB 200 kB +20 B (+0.0%)
  • @firebase/firestore-lite

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 64.9 kB 64.9 kB +20 B (+0.0%)
    main 143 kB 143 kB +42 B (+0.0%)
    module 64.9 kB 64.9 kB +20 B (+0.0%)
    react-native 65.1 kB 65.1 kB +20 B (+0.0%)
  • @firebase/firestore/bundle

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 266 kB 266 kB -5 B (-0.0%)
    esm2017 155 kB 155 kB -5 B (-0.0%)
    main 526 kB 526 kB -1 B (-0.0%)
    module 266 kB 266 kB -5 B (-0.0%)
    react-native 155 kB 155 kB -5 B (-0.0%)
  • @firebase/firestore/memory

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 196 kB 196 kB -5 B (-0.0%)
    esm2017 152 kB 152 kB -5 B (-0.0%)
    main 324 kB 324 kB -1 B (-0.0%)
    module 196 kB 196 kB -5 B (-0.0%)
    react-native 152 kB 152 kB -5 B (-0.0%)
  • @firebase/firestore/memory-bundle

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 205 kB 205 kB -5 B (-0.0%)
    esm2017 155 kB 155 kB -5 B (-0.0%)
    main 321 kB 321 kB -1 B (-0.0%)
    module 205 kB 205 kB -5 B (-0.0%)
    react-native 155 kB 155 kB -5 B (-0.0%)
  • @firebase/functions

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 9.87 kB 9.90 kB +29 B (+0.3%)
    esm2017 7.69 kB 7.71 kB +21 B (+0.3%)
    main 10.3 kB 10.3 kB +29 B (+0.3%)
    module 9.87 kB 9.90 kB +29 B (+0.3%)
  • @firebase/remote-config

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 22.4 kB 22.4 kB +29 B (+0.1%)
    esm2017 17.4 kB 17.4 kB +21 B (+0.1%)
    main 22.9 kB 23.0 kB +29 B (+0.1%)
    module 22.4 kB 22.4 kB +29 B (+0.1%)
  • @firebase/storage

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 60.2 kB 60.2 kB +29 B (+0.0%)
    esm2017 51.9 kB 51.9 kB +21 B (+0.0%)
    main 60.7 kB 60.7 kB +29 B (+0.0%)
    module 60.2 kB 60.2 kB +29 B (+0.0%)
  • @firebase/storage/exp

    Type Base (b6080a8) Head (dcf9fee) Diff
    browser 49.2 kB 49.2 kB +21 B (+0.0%)
    main 50.0 kB 50.0 kB +21 B (+0.0%)
    module 49.2 kB 49.2 kB +21 B (+0.0%)
  • firebase

    Click to show 15 binary size changes.
    Type Base (b6080a8) Head (dcf9fee) Diff
    firebase-analytics.js 35.6 kB 35.6 kB +6 B (+0.0%)
    firebase-app.js 19.9 kB 20.4 kB +579 B (+2.9%)
    firebase-auth.js 177 kB 177 kB +20 B (+0.0%)
    firebase-database.js 179 kB 179 kB +17 B (+0.0%)
    firebase-firestore.js 303 kB 303 kB -11 B (-0.0%)
    firebase-firestore.memory.js 243 kB 243 kB -11 B (-0.0%)
    firebase-functions.js 10.1 kB 10.2 kB +22 B (+0.2%)
    firebase-installations.js 19.1 kB 19.1 kB +11 B (+0.1%)
    firebase-messaging.js 40.8 kB 40.9 kB +103 B (+0.3%)
    firebase-performance-standalone.es2017.js 71.9 kB 72.3 kB +455 B (+0.6%)
    firebase-performance-standalone.js 47.8 kB 48.4 kB +591 B (+1.2%)
    firebase-performance.js 38.2 kB 38.2 kB +11 B (+0.0%)
    firebase-remote-config.js 36.7 kB 36.8 kB +30 B (+0.1%)
    firebase-storage.js 39.8 kB 39.9 kB +22 B (+0.1%)
    firebase.js 839 kB 840 kB +203 B (+0.0%)

Test Logs

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 9, 2021

Size Analysis Report

Affected Products

Diffs between base commit (b6080a8) and head commit (dcf9fee) are too large (198,650 characters) to display.

Please check below links to see details from the original test log.

@schmidt-sebastian schmidt-sebastian removed their assignment Mar 9, 2021
Copy link
Contributor

@jposuna jposuna left a comment

Choose a reason for hiding this comment

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

LGTM for perf, thanks!

@github-actions
Copy link
Contributor

Changeset File Check ⚠️

  • Warning: This PR modifies files in the following packages but they have not been included in the changeset file:

    • @firebase/analytics-compat
    • @firebase/auth-exp
    • @firebase/functions-exp
    • @firebase/performance-exp
    • @firebase/remote-config-compat
    • @firebase/remote-config-exp

    Make sure this was intentional.

  • Package @firebase/component has a minor bump which requires an additional line to bump the main "firebase" package to minor.

@Feiyang1 Feiyang1 merged commit 5c1a83e into master Mar 12, 2021
@Feiyang1 Feiyang1 deleted the fei-cf-update branch March 12, 2021 21:36
@google-oss-bot google-oss-bot mentioned this pull request Mar 16, 2021
@firebase firebase locked and limited conversation to collaborators Apr 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants