Skip to content

Make node and browser entry points identical #5318

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 7 commits into from
Aug 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions packages-exp/auth-compat-exp/src/popup_redirect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ import { FirebaseApp } from '@firebase/app-compat';
use(sinonChai);

describe('popup_redirect/CompatPopupRedirectResolver', () => {
// Do not run these tests in node; in node, this resolver
// is never instantiated.
if (typeof window === 'undefined') {
console.log(
'Skipping popup/redirect resolver tests in non-browser environment'
);
return;
}

let compatResolver: CompatPopupRedirectResolver;
let auth: exp.AuthImpl;

Expand Down Expand Up @@ -75,9 +84,11 @@ describe('popup_redirect/CompatPopupRedirectResolver', () => {

beforeEach(() => {
underlyingResolver = sinon.createStubInstance(FakeResolver);
((compatResolver as unknown) as {
underlyingResolver: exp.PopupRedirectResolverInternal;
}).underlyingResolver = underlyingResolver;
(
compatResolver as unknown as {
underlyingResolver: exp.PopupRedirectResolverInternal;
}
).underlyingResolver = underlyingResolver;
provider = new exp.GoogleAuthProvider();
});

Expand Down
21 changes: 12 additions & 9 deletions packages-exp/auth-compat-exp/src/popup_redirect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@ import * as exp from '@firebase/auth-exp/internal';
import { _isCordova, _isLikelyCordova } from './platform';

const _assert: typeof exp._assert = exp._assert;
const BROWSER_RESOLVER: exp.PopupRedirectResolverInternal = exp._getInstance(
exp.browserPopupRedirectResolver
);
const CORDOVA_RESOLVER: exp.PopupRedirectResolverInternal = exp._getInstance(
exp.cordovaPopupRedirectResolver
);

/** Platform-agnostic popup-redirect resolver */
export class CompatPopupRedirectResolver
implements exp.PopupRedirectResolverInternal {
implements exp.PopupRedirectResolverInternal
{
// Create both resolvers for dynamic resolution later
private readonly browserResolver: exp.PopupRedirectResolverInternal =
exp._getInstance(exp.browserPopupRedirectResolver);
private readonly cordovaResolver: exp.PopupRedirectResolverInternal =
exp._getInstance(exp.cordovaPopupRedirectResolver);
// The actual resolver in use: either browserResolver or cordovaResolver.
private underlyingResolver: exp.PopupRedirectResolverInternal | null = null;
_redirectPersistence = exp.browserSessionPersistence;

Expand Down Expand Up @@ -85,7 +86,7 @@ export class CompatPopupRedirectResolver
}

get _shouldInitProactively(): boolean {
return _isLikelyCordova() || BROWSER_RESOLVER._shouldInitProactively;
return _isLikelyCordova() || this.browserResolver._shouldInitProactively;
}

private get assertedUnderlyingResolver(): exp.PopupRedirectResolverInternal {
Expand All @@ -101,6 +102,8 @@ export class CompatPopupRedirectResolver
// We haven't yet determined whether or not we're in Cordova; go ahead
// and determine that state now.
const isCordova = await _isCordova();
this.underlyingResolver = isCordova ? CORDOVA_RESOLVER : BROWSER_RESOLVER;
this.underlyingResolver = isCordova
? this.cordovaResolver
: this.browserResolver;
}
}
3 changes: 1 addition & 2 deletions packages-exp/auth-exp/index.cordova.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ import { initializeAuth } from './src';
import { registerAuth } from './src/core/auth/register';
import { ClientPlatform } from './src/core/util/version';

// Core functionality shared by all clients
export * from './src';
export * from './index.shared';

// Cordova also supports indexedDB / browserSession / browserLocal
export { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';
Expand Down
49 changes: 3 additions & 46 deletions packages-exp/auth-exp/index.node.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* @license
* Copyright 2017 Google LLC
* Copyright 2021 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,48 +15,5 @@
* limitations under the License.
*/

/**
* This is the file that people using Node.js will actually import. You should
* only include this file if you have something specific about your
* implementation that mandates having a separate entrypoint. Otherwise you can
* just use index.browser.ts
*/

import * as fetchImpl from 'node-fetch';

import { FirebaseApp, getApp, _getProvider } from '@firebase/app-exp';
import { Auth } from './src/model/public_types';

import { initializeAuth } from './src';
import { registerAuth } from './src/core/auth/register';
import { FetchProvider } from './src/core/util/fetch_provider';
import { ClientPlatform } from './src/core/util/version';

// Initialize the fetch polyfill, the types are slightly off so just cast and hope for the best
FetchProvider.initialize(
fetchImpl.default as unknown as typeof fetch,
fetchImpl.Headers as unknown as typeof Headers,
fetchImpl.Response as unknown as typeof Response
);

// Core functionality shared by all clients
export * from './src';
export {
FactorId,
ProviderId,
SignInMethod,
OperationType,
ActionCodeOperation
} from './src/model/enum_maps';

export function getAuth(app: FirebaseApp = getApp()): Auth {
const provider = _getProvider(app, 'auth-exp');

if (provider.isInitialized()) {
return provider.getImmediate();
}

return initializeAuth(app);
}

registerAuth(ClientPlatform.NODE);
// Node entry point is identical to browser entry point
export * from './index';
2 changes: 1 addition & 1 deletion packages-exp/auth-exp/index.rn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { ClientPlatform } from './src/core/util/version';
import { getReactNativePersistence } from './src/platform_react_native/persistence/react_native';

// Core functionality shared by all clients
export * from './src';
export * from './index.shared';

/**
* An implementation of {@link Persistence} of type 'LOCAL' for use in React
Expand Down
31 changes: 31 additions & 0 deletions packages-exp/auth-exp/index.shared.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* @license
* Copyright 2021 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

// Public types
export * from './src/model/public_types';

// Helper maps (not used internally)
export {
FactorId,
ProviderId,
SignInMethod,
OperationType,
ActionCodeOperation
} from './src/model/enum_maps';

// Core functionality shared by all clients
export * from './src';
84 changes: 37 additions & 47 deletions packages-exp/auth-exp/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,6 @@
* limitations under the License.
*/

import { FirebaseApp, getApp, _getProvider } from '@firebase/app-exp';

import { initializeAuth } from './src';
import { registerAuth } from './src/core/auth/register';
import { ClientPlatform } from './src/core/util/version';
import { browserLocalPersistence } from './src/platform_browser/persistence/local_storage';
import { browserSessionPersistence } from './src/platform_browser/persistence/session_storage';
import { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';
import { browserPopupRedirectResolver } from './src/platform_browser/popup_redirect';
import { Auth } from './src/model/public_types';

// Public types
export * from './src/model/public_types';

Expand All @@ -47,64 +36,65 @@ export {
// Core functionality shared by all clients
export * from './src';

// Additional DOM dependend functionality
// Additional DOM dependend functionality; we need to import and then
// export separately so that the rollup alias will work (for aliasing these
Copy link
Member

Choose a reason for hiding this comment

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

interesting. Is it WAI, or a bug in the plugin? If it's a bug, it may be worth linking to the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's WAI I think. It uses resolveId which is import statements according to rollup. See: https://github.com/rollup/rollup-plugin-alias/blob/master/src/index.js

// imports in node environments to no-ops and errors... see
// src/platform_node/index.ts).

// persistence
export { browserLocalPersistence } from './src/platform_browser/persistence/local_storage';
export { browserSessionPersistence } from './src/platform_browser/persistence/session_storage';
export { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';
import { browserLocalPersistence } from './src/platform_browser/persistence/local_storage';
import { browserSessionPersistence } from './src/platform_browser/persistence/session_storage';
import { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';

// providers
export { PhoneAuthProvider } from './src/platform_browser/providers/phone';
import { PhoneAuthProvider } from './src/platform_browser/providers/phone';

// strategies
export {
import {
signInWithPhoneNumber,
linkWithPhoneNumber,
reauthenticateWithPhoneNumber,
updatePhoneNumber
} from './src/platform_browser/strategies/phone';
export {
import {
signInWithPopup,
linkWithPopup,
reauthenticateWithPopup
} from './src/platform_browser/strategies/popup';
export {
import {
signInWithRedirect,
linkWithRedirect,
reauthenticateWithRedirect,
getRedirectResult
} from './src/platform_browser/strategies/redirect';

export { RecaptchaVerifier } from './src/platform_browser/recaptcha/recaptcha_verifier';
export { browserPopupRedirectResolver } from './src/platform_browser/popup_redirect';
import { RecaptchaVerifier } from './src/platform_browser/recaptcha/recaptcha_verifier';
import { browserPopupRedirectResolver } from './src/platform_browser/popup_redirect';

// MFA
export { PhoneMultiFactorGenerator } from './src/platform_browser/mfa/assertions/phone';

/**
* Returns the Auth instance associated with the provided {@link @firebase/app#FirebaseApp}.
* If no instance exists, initializes an Auth instance with platform-specific default dependencies.
*
* @param app - The Firebase App.
*
* @public
*/
export function getAuth(app: FirebaseApp = getApp()): Auth {
const provider = _getProvider(app, 'auth-exp');
import { PhoneMultiFactorGenerator } from './src/platform_browser/mfa/assertions/phone';

if (provider.isInitialized()) {
return provider.getImmediate();
}
// Initialization and registration of Auth
import { getAuth } from './src/platform_browser';

return initializeAuth(app, {
popupRedirectResolver: browserPopupRedirectResolver,
persistence: [
indexedDBLocalPersistence,
browserLocalPersistence,
browserSessionPersistence
]
});
}

registerAuth(ClientPlatform.BROWSER);
export {
browserLocalPersistence,
browserSessionPersistence,
indexedDBLocalPersistence,
PhoneAuthProvider,
signInWithPhoneNumber,
linkWithPhoneNumber,
reauthenticateWithPhoneNumber,
updatePhoneNumber,
signInWithPopup,
linkWithPopup,
reauthenticateWithPopup,
signInWithRedirect,
linkWithRedirect,
reauthenticateWithRedirect,
getRedirectResult,
RecaptchaVerifier,
browserPopupRedirectResolver,
PhoneMultiFactorGenerator,
getAuth
};
2 changes: 1 addition & 1 deletion packages-exp/auth-exp/index.webworker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { ClientPlatform } from './src/core/util/version';
import { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';

// Core functionality shared by all clients
export * from './src';
export * from './index.shared';

// persistence
export { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';
Expand Down
10 changes: 5 additions & 5 deletions packages-exp/auth-exp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@
"test:cordova": "karma start --single-run --cordova",
"test:cordova:debug": "karma start --auto-watch --cordova",
"test:node": "run-s test:node:unit test:node:integration:local",
"test:node:unit": "node ./scripts/run-node-tests.js",
"test:node:integration": "node ./scripts/run-node-tests.js --integration",
"test:node:integration:local": "node ./scripts/run-node-tests.js --integration --local",
"test:webdriver": "rollup -c test/integration/webdriver/static/rollup.config.js && node ./scripts/run-node-tests.js --webdriver",
"test:node:unit": "ts-node -O '{\"module\": \"commonjs\", \"target\": \"es6\"}' scripts/run_node_tests.ts",
"test:node:integration": "ts-node -O '{\"module\": \"commonjs\", \"target\": \"es6\"}' scripts/run_node_tests.ts --integration",
"test:node:integration:local": "ts-node -O '{\"module\": \"commonjs\", \"target\": \"es6\"}' scripts/run_node_tests.ts --integration --local",
"test:webdriver": "rollup -c test/integration/webdriver/static/rollup.config.js && ts-node -O '{\"module\": \"commonjs\", \"target\": \"es6\"}' scripts/run_node_tests.ts --webdriver",
"api-report": "api-extractor run --local --verbose && ts-node-script ../../repo-scripts/prune-dts/prune-dts.ts --input dist/auth-exp-public.d.ts --output dist/auth-exp-public.d.ts",
"predoc": "node ../../scripts/exp/remove-exp.js temp",
"doc": "api-documenter markdown --input temp --output docs",
Expand Down Expand Up @@ -85,4 +85,4 @@
"reportDir": "./coverage/node"
},
"esm5": "dist/esm5/index.js"
}
}
18 changes: 17 additions & 1 deletion packages-exp/auth-exp/rollup.config.shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import strip from '@rollup/plugin-strip';
import typescriptPlugin from 'rollup-plugin-typescript2';
import json from '@rollup/plugin-json';
import alias from '@rollup/plugin-alias';
import typescript from 'typescript';
import pkg from './package.json';
import { importPathTransformer } from '../../scripts/exp/ts-transform-import-path';
Expand All @@ -37,6 +38,21 @@ const commonPlugins = [
})
];

/**
* Node has the same entry point as browser, but browser-specific exports
* are turned into either no-ops or errors. See src/platform_node/index.ts for
* more info. This regex tests explicitly ./src/platform_browser so that the
* only impacted file is the main index.ts
*/
const nodeAliasPlugin = alias({
entries: [
{
find: /^\.\/src\/platform_browser(\/.*)?$/,
replacement: `./src/platform_node`
}
]
});

export function getConfig({ isReleaseBuild }) {
/**
* ES5 Builds
Expand Down Expand Up @@ -90,7 +106,7 @@ export function getConfig({ isReleaseBuild }) {
internal: 'internal/index.ts'
},
output: [{ dir: 'dist/node', format: 'cjs', sourcemap: true }],
plugins: es5BuildPlugins,
plugins: [nodeAliasPlugin, ...es5BuildPlugins],
external: id => deps.some(dep => id === dep || id.startsWith(`${dep}/`))
},
/**
Expand Down
Loading