Copy demo code over to auth-compat-layer, fix our build scripts #3563

avolkovi · 2020-08-04T23:32:40Z

No description provided.

changeset-bot · 2020-08-04T23:32:42Z

💥 No Changeset

Latest commit: d5efb21

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

packages-exp/auth-compat-exp/demo/.firebaserc

packages-exp/auth-compat-exp/demo/rollup.config.js

packages-exp/auth-compat-exp/src/phone_multi_factor_generator.ts

packages-exp/auth-exp/internal/index.ts

packages-exp/auth-exp/rollup.config.internal.js

* Copy demo code over to auth-compat-layer, fix our build scripts * Add more functionality to compat layer * PR Feedback * PR Feedback

Feiyang1

I'm too slow at reviewing! Left some comments, PTAL

Feiyang1 · 2020-08-06T17:30:09Z

packages-exp/auth-compat-exp/demo/package.json

+  "dependencies": {
+    "@firebase/logger": "^0.2.2",
+    "@firebase/util": "^0.2.44",
+    "lerna": "^3.22.1",


Use exact version for @firebase packages. Also, I guess lerna is a mistake here?

Feiyang1 · 2020-08-06T17:32:36Z

packages-exp/auth-compat-exp/demo/package.json

+  "bugs": {
+    "url": "https://github.com/firebase/firebase-js-sdk/issues"
+  },
+  "typings": "dist/index.d.ts",


Nit: You don't need a typing file for an app, so delete this line.

Feiyang1 · 2020-08-06T17:39:13Z

packages-exp/auth-compat-exp/demo/rollup.config.js

+      format: 'umd',
+      name: 'firebase'
+    },
+    plugins: [...plugins]


nit: can shorten to just plugins

{ input: '...', plugins }

I think the build step for the demo app can be removed (rollup, tsconfig, package.json) once we add firebase-compat where we build the scripts that you can copy here.

Feiyang1 · 2020-08-06T17:51:36Z

packages-exp/auth-compat-exp/index.ts

@@ -74,7 +78,7 @@ function registerAuth(instance: _FirebaseNamespace): void {
      .setMultipleInstances(false)
  );

-  instance.registerVersion(name, version);
+  instance.registerVersion('auth', version);


instance.registerVersion('auth', version, 'compat'); to differentiate the modular SDK and the compat SDK in metrics.

Feiyang1 · 2020-08-06T19:45:33Z

packages-exp/auth-compat-exp/package.json

@@ -38,11 +37,12 @@
  },
  "license": "Apache-2.0",
  "devDependencies": {
+    "@firebase/app-exp": "0.x",


We probably want to move all peerDependencies to dependencies using exact versions except for @firebase/app(-compat). We will ask compat users to install firebase-compat without needing to install firebase.

I don't think auth uses @firebase/installations, so it should be removed.

Feiyang1 · 2020-08-06T20:44:34Z

packages-exp/auth-compat-exp/rollup.config.js

-  /**
-   * React Native Builds
-   */
-  {


Do we still need a React native build?

Feiyang1 · 2020-08-06T21:05:47Z

packages-exp/auth-compat-exp/src/auth.ts

  convertCredential
 } from './user_credential';

-export class Auth extends AuthImplCompat<User> implements compat.FirebaseAuth {
+export class Auth extends impl.AuthImplCompat<User>


Is it possible to get an instance of modular Auth from the compat Auth, and similarly for other things, e.g. User? Or is AuthImplCompat compatible with the modular Auth?
We want to allow people to upgrade to the modular SDK piece meal, meaning they can use the compat SDK and upgrade part of their auth code to use the modular SDK. Something like:

import * as firebase from 'firebase-compat'; import 'firebase-compat/auth' ; import { signInWithEmailAndPassword } from 'firebase/auth'; const authCompat = firebase.auth(); // raw() returns an instance of modular Auth. const auth = authCompat.raw(); signInWithEmailAndPasswood(auth, .... );

Feiyang1 · 2020-08-06T21:16:49Z

packages-exp/auth-exp/package.json

  "author": "Firebase <[email protected]> (https://firebase.google.com/)",
  "main": "dist/index.node.cjs.js",
+  "main-esm2017": "dist/index.node.esm2017.js",
+  "react-native": "dist/index.rn.esm2017.js",
  "browser": "dist/index.cjs.js",


browser and module should both be esm2017.

Feiyang1 · 2020-08-06T21:17:31Z

packages-exp/auth-exp/package.json

+    "@firebase/logger": "^0.2.6",
+    "@firebase/util": "^0.3.0",


use exact version for @firebase dependencies

Feiyang1 · 2020-08-06T21:26:01Z

packages-exp/auth-exp/rollup.config.internal.js

+  json()
+];
+
+const allBuilds = [


We should combine the internal build into the main build, otherwise they won't share code. We will need to do the same for other submodules you will add, e.g. auth/cordova. It might be easier that I make the change and create a PR against your branch.

* Copy demo code over to auth-compat-layer, fix our build scripts * Add more functionality to compat layer * PR Feedback * PR Feedback

avolkovi requested a review from sam-gc August 4, 2020 23:32

avolkovi requested review from Feiyang1, hiranya911 and hsubox76 as code owners August 4, 2020 23:32

avolkovi assigned Feiyang1 Aug 4, 2020

Base automatically changed from avolkovi/polyfill to auth-next August 5, 2020 15:49

avolkovi requested review from bojeil-google and scottcrossen as code owners August 5, 2020 15:49

Copy demo code over to auth-compat-layer, fix our build scripts

bc5328d

avolkovi force-pushed the avolkovi/polyfill-demo branch 3 times, most recently from 86b66c7 to 0a0f517 Compare August 5, 2020 19:09

Add more functionality to compat layer

82b7bae

avolkovi force-pushed the avolkovi/polyfill-demo branch from 0a0f517 to 82b7bae Compare August 5, 2020 19:10

sam-gc reviewed Aug 5, 2020

View reviewed changes

packages-exp/auth-compat-exp/demo/.firebaserc Outdated Show resolved Hide resolved

PR Feedback

11bd387

sam-gc approved these changes Aug 5, 2020

View reviewed changes

PR Feedback

d5efb21

avolkovi merged commit de2b43b into auth-next Aug 6, 2020

avolkovi deleted the avolkovi/polyfill-demo branch August 6, 2020 17:27

avolkovi added a commit that referenced this pull request Aug 6, 2020

Copy demo code over to auth-compat-layer, fix our build scripts (#3563)

745776f

* Copy demo code over to auth-compat-layer, fix our build scripts * Add more functionality to compat layer * PR Feedback * PR Feedback

Feiyang1 reviewed Aug 6, 2020

View reviewed changes

avolkovi added a commit that referenced this pull request Aug 10, 2020

Copy demo code over to auth-compat-layer, fix our build scripts (#3563)

d2c3e30

* Copy demo code over to auth-compat-layer, fix our build scripts * Add more functionality to compat layer * PR Feedback * PR Feedback

sam-gc pushed a commit that referenced this pull request Sep 1, 2020

Copy demo code over to auth-compat-layer, fix our build scripts (#3563)

9b30f7c

* Copy demo code over to auth-compat-layer, fix our build scripts * Add more functionality to compat layer * PR Feedback * PR Feedback

firebase locked and limited conversation to collaborators Sep 6, 2020

Copy demo code over to auth-compat-layer, fix our build scripts #3563

Copy demo code over to auth-compat-layer, fix our build scripts #3563

Uh oh!

Conversation

avolkovi commented Aug 4, 2020

Uh oh!

changeset-bot bot commented Aug 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💥 No Changeset

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Feiyang1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

changeset-bot bot commented Aug 4, 2020 •

edited

Loading