Skip to content

V11 release removal of node-fetch and undici dependencies #8492

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 28 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
4bdab6f
Removed node-fetch from rules-unit-testing
DellaBitta Sep 10, 2024
45723bb
Draft emulator impl not quite working.
DellaBitta Sep 11, 2024
5ee8eb7
Wrap the emulator download in a promise
DellaBitta Sep 11, 2024
84f0770
Update yarn.lock
DellaBitta Sep 12, 2024
afea52d
emulator.ts reject/resolve fix. formatting.
DellaBitta Sep 12, 2024
79753b2
Update emaultor.ts with chmod operation.
DellaBitta Sep 12, 2024
ba32125
emulator.ts remove setBinaryPath & add comments.
DellaBitta Sep 12, 2024
6fdd758
Remove undici from messaging testing
DellaBitta Sep 12, 2024
0601594
Remove undici from auth source and package.json
DellaBitta Sep 12, 2024
2c4a46e
auth format
DellaBitta Sep 12, 2024
3b93ffe
repo-scripts
DellaBitta Sep 12, 2024
97eb7a3
Auth: remove custom webpack step for undici
DellaBitta Sep 12, 2024
9c010b2
Remove undici from firestore
DellaBitta Sep 13, 2024
69d789c
Lint fix.
DellaBitta Sep 13, 2024
c8c64e7
Remove fetch from functions
DellaBitta Sep 13, 2024
b16903b
fix build
DellaBitta Sep 13, 2024
0c05f70
Fixed build?
DellaBitta Sep 13, 2024
5077ae3
format
DellaBitta Sep 13, 2024
7a6691b
Remove undici from storage
DellaBitta Sep 13, 2024
033c4dc
Remove undici from changelog generator
DellaBitta Sep 13, 2024
9aa1f75
Fix storage test for node
DellaBitta Sep 13, 2024
f35d2a0
Remove no longer required eslint-disable comments.
DellaBitta Sep 16, 2024
1e25ff2
Functions export public-types
DellaBitta Sep 16, 2024
7acb14e
Auth - remove superfluous casting.
DellaBitta Sep 19, 2024
498bc7b
Fix hanging promise in emulator download code
DellaBitta Sep 19, 2024
10a6149
Remove fetch casting in auth-compat
DellaBitta Sep 20, 2024
10e089e
Changeset
DellaBitta Sep 20, 2024
d517b14
Changeset 2
DellaBitta Sep 20, 2024
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
13 changes: 13 additions & 0 deletions .changeset/plenty-beers-decide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@firebase/rules-unit-testing': patch
'@firebase/firestore-compat': patch
'@firebase/functions-compat': patch
'@firebase/storage-compat': patch
'@firebase/auth-compat': patch
'@firebase/firestore': patch
'@firebase/functions': patch
'@firebase/storage': patch
'@firebase/auth': patch
---

Removed dependency on undici and node-fetch in our node bundles, replacing them with the native fetch implementation.
1 change: 0 additions & 1 deletion integration/messaging/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
"express": "4.19.2",
"geckodriver": "2.0.4",
"mocha": "9.2.2",
"undici": "6.19.7",
"selenium-assistant": "6.1.1"
},
"engines": {
Expand Down
3 changes: 1 addition & 2 deletions integration/messaging/test/utils/sendMessage.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/

const undici = require('undici');
const FCM_SEND_ENDPOINT = 'https://fcm.googleapis.com/fcm/send';
// Rotatable fcm server key. It's generally a bad idea to expose server keys. The reason is to
// simplify testing process (no need to implement server side decryption of git secret). The
Expand All @@ -28,7 +27,7 @@ module.exports = async payload => {
'Requesting to send an FCM message with payload: ' + JSON.stringify(payload)
);

const response = await undici.fetch(FCM_SEND_ENDPOINT, {
const response = await fetch(FCM_SEND_ENDPOINT, {
method: 'POST',
body: JSON.stringify(payload),
headers: {
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@
"tslint": "6.1.3",
"typedoc": "0.16.11",
"typescript": "4.7.4",
"undici": "6.19.7",
"watch": "1.0.2",
"webpack": "5.76.0",
"yargs": "17.7.2"
Expand Down
11 changes: 1 addition & 10 deletions packages/auth-compat/index.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,6 @@
*/
export * from './index';
import { FetchProvider } from '@firebase/auth/internal';
import {
fetch as undiciFetch,
Headers as undiciHeaders,
Response as undiciResponse
} from 'undici';
import './index';

FetchProvider.initialize(
undiciFetch as unknown as typeof fetch,
undiciHeaders as unknown as typeof Headers,
undiciResponse as unknown as typeof Response
);
FetchProvider.initialize(fetch, Headers, Response);
12 changes: 0 additions & 12 deletions packages/auth-compat/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,6 @@ module.exports = function (config) {
// frameworks to use
// available frameworks: https://npmjs.org/browse/keyword/karma-adapter
frameworks: ['mocha'],
// undici is a fetch polyfill that test helpers call for Node tests, and browser tests should
// ignore its import to avoid compilation errors in those test helpers.
webpack: {
...webpackBase,
resolve: {
...webpackBase.resolve,
alias: {
'undici': false
}
}
},

client: Object.assign({}, karmaBase.client, getClientConfig())
});

Expand Down
1 change: 0 additions & 1 deletion packages/auth-compat/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
"@firebase/auth-types": "0.12.2",
"@firebase/component": "0.6.8",
"@firebase/util": "1.9.7",
"undici": "6.19.7",
"tslib": "^2.1.0"
},
"license": "Apache-2.0",
Expand Down
11 changes: 0 additions & 11 deletions packages/auth/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,6 @@ module.exports = function (config) {
// frameworks to use
// available frameworks: https://npmjs.org/browse/keyword/karma-adapter
frameworks: ['mocha'],
// undici is a fetch polyfill that test helpers call for Node tests, and browser tests should
// ignore its import to avoid compilation errors in those test helpers.
webpack: {
...webpackBase,
resolve: {
...webpackBase.resolve,
alias: {
'undici': false
}
}
},
client: Object.assign({}, karmaBase.client, getClientConfig(argv))
});

Expand Down
1 change: 0 additions & 1 deletion packages/auth/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@
"@firebase/component": "0.6.8",
"@firebase/logger": "0.4.2",
"@firebase/util": "1.9.7",
"undici": "6.19.7",
"tslib": "^2.1.0"
},
"license": "Apache-2.0",
Expand Down
11 changes: 1 addition & 10 deletions packages/auth/src/platform_node/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,9 @@ import { AuthImpl } from '../core/auth/auth_impl';

import { FetchProvider } from '../core/util/fetch_provider';
import { getDefaultEmulatorHost } from '@firebase/util';
import {
fetch as undiciFetch,
Headers as undiciHeaders,
Response as undiciResponse
} from 'undici';

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

// First, we set up the various platform-specific features for Node (register
// the version and declare the Node getAuth function)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/

import { fetch as undiciFetch, RequestInit as undiciRequestInit } from 'undici';
import { getAppConfig, getEmulatorUrl } from './settings';

export interface VerificationSession {
Expand Down Expand Up @@ -88,9 +87,8 @@ function doFetch(url: string, request?: RequestInit): ReturnType<typeof fetch> {
if (typeof document !== 'undefined') {
return fetch(url, request);
} else {
return undiciFetch(
url,
request as undiciRequestInit
) as unknown as ReturnType<typeof fetch>;
return fetch(url, request as RequestInit) as unknown as ReturnType<
typeof fetch
>;
}
}
1 change: 0 additions & 1 deletion packages/firestore/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@
"@firebase/webchannel-wrapper": "1.0.1",
"@grpc/grpc-js": "~1.9.0",
"@grpc/proto-loader": "^0.7.8",
"undici": "6.19.7",
"tslib": "^2.1.0"
},
"peerDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/src/platform/browser_lite/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@ export { newConnectivityMonitor } from '../browser/connection';

/** Initializes the HTTP connection for the REST API. */
export function newConnection(databaseInfo: DatabaseInfo): Connection {
return new FetchConnection(databaseInfo, fetch.bind(null));
return new FetchConnection(databaseInfo);
}
14 changes: 1 addition & 13 deletions packages/firestore/src/platform/browser_lite/fetch_connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/

import { Token } from '../../api/credentials';
import { DatabaseInfo } from '../../core/database_info';
import { Stream } from '../../remote/connection';
import { RestConnection } from '../../remote/rest_connection';
import { mapCodeFromHttpStatus } from '../../remote/rpc_error';
Expand All @@ -28,17 +27,6 @@ import { StringMap } from '../../util/types';
* (e.g. `fetch` or a polyfill).
*/
export class FetchConnection extends RestConnection {
/**
* @param databaseInfo - The connection info.
* @param fetchImpl - `fetch` or a Polyfill that implements the fetch API.
*/
constructor(
databaseInfo: DatabaseInfo,
private readonly fetchImpl: typeof fetch
) {
super(databaseInfo);
Copy link
Contributor Author

@DellaBitta DellaBitta Sep 16, 2024

Choose a reason for hiding this comment

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

This consructor is removed because it would now have the same signature as RestConnection's constructor.

}

openStream<Req, Resp>(
rpcName: string,
token: Token | null
Expand All @@ -56,7 +44,7 @@ export class FetchConnection extends RestConnection {
let response: Response;

try {
response = await this.fetchImpl(url, {
response = await fetch(url, {
method: 'POST',
headers,
body: requestJson
Expand Down
7 changes: 1 addition & 6 deletions packages/firestore/src/platform/node_lite/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
* limitations under the License.
*/

import { fetch as undiciFetch } from 'undici';

import { DatabaseInfo } from '../../core/database_info';
import { Connection } from '../../remote/connection';
import { FetchConnection } from '../browser_lite/fetch_connection';
Expand All @@ -25,8 +23,5 @@ export { newConnectivityMonitor } from '../browser/connection';

/** Initializes the HTTP connection for the REST API. */
export function newConnection(databaseInfo: DatabaseInfo): Connection {
// undici is meant to be API compatible with `fetch`, but its type doesn't
// match 100%.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return new FetchConnection(databaseInfo, undiciFetch as any);
return new FetchConnection(databaseInfo);
}
13 changes: 1 addition & 12 deletions packages/functions/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,7 @@ module.exports = function (config) {
files,
// frameworks to use
// available frameworks: https://npmjs.org/browse/keyword/karma-adapter
frameworks: ['mocha'],
// undici is a fetch polyfill that test helpers call for Node tests, and browser tests should
// ignore its import to avoid compilation errors in those test helpers.
webpack: {
...webpackBase,
resolve: {
...webpackBase.resolve,
alias: {
'undici': false
}
}
}
frameworks: ['mocha']
});

config.set(karmaConfig);
Expand Down
1 change: 0 additions & 1 deletion packages/functions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@
"@firebase/auth-interop-types": "0.2.3",
"@firebase/app-check-interop-types": "0.3.2",
"@firebase/util": "1.9.7",
"undici": "6.19.7",
"tslib": "^2.1.0"
},
"nyc": {
Expand Down
8 changes: 2 additions & 6 deletions packages/functions/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ const APP_CHECK_INTERNAL_NAME: AppCheckInternalComponentName =
const MESSAGING_INTERNAL_NAME: MessagingInternalComponentName =
'messaging-internal';

export function registerFunctions(
fetchImpl: typeof fetch,
variant?: string
): void {
export function registerFunctions(variant?: string): void {
const factory: InstanceFactory<'functions'> = (
container: ComponentContainer,
{ instanceIdentifier: regionOrCustomDomain }
Expand All @@ -55,8 +52,7 @@ export function registerFunctions(
authProvider,
messagingProvider,
appCheckProvider,
regionOrCustomDomain,
fetchImpl
regionOrCustomDomain
);
};

Expand Down
5 changes: 2 additions & 3 deletions packages/functions/src/index.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@
* limitations under the License.
*/
import { registerFunctions } from './config';
import { fetch as undiciFetch } from 'undici';

export * from './api';
export * from './public-types';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
registerFunctions(undiciFetch as any, 'node');
registerFunctions('node');
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I don't think we actually need a separate functions Node bundle anymore. Maybe that can be a separate PR though. The only thing different is that it labels the bundle "node" for platform logging, which is pointless if it's the same bundle. I guess it tells us how many node users are using functions. Also it doesn't export the types from public-types which is probably a mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add the export.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We spoke offline about removing the separate node bundle, will be done in a different PR.

2 changes: 1 addition & 1 deletion packages/functions/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ import { registerFunctions } from './config';
export * from './api';
export * from './public-types';

registerFunctions(fetch.bind(self));
registerFunctions();
10 changes: 4 additions & 6 deletions packages/functions/src/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ export class FunctionsService implements _FirebaseService {
authProvider: Provider<FirebaseAuthInternalName>,
messagingProvider: Provider<MessagingInternalComponentName>,
appCheckProvider: Provider<AppCheckInternalComponentName>,
regionOrCustomDomain: string = DEFAULT_REGION,
readonly fetchImpl: typeof fetch
regionOrCustomDomain: string = DEFAULT_REGION
) {
this.contextProvider = new ContextProvider(
authProvider,
Expand Down Expand Up @@ -212,14 +211,13 @@ export function httpsCallableFromURL<RequestData, ResponseData>(
async function postJSON(
url: string,
body: unknown,
headers: { [key: string]: string },
fetchImpl: typeof fetch
headers: { [key: string]: string }
): Promise<HttpResponse> {
headers['Content-Type'] = 'application/json';

let response: Response;
try {
response = await fetchImpl(url, {
response = await fetch(url, {
method: 'POST',
body: JSON.stringify(body),
headers
Expand Down Expand Up @@ -296,7 +294,7 @@ async function callAtURL(

const failAfterHandle = failAfter(timeout);
const response = await Promise.race([
postJSON(url, body, headers, functionsInstance.fetchImpl),
postJSON(url, body, headers),
failAfterHandle.promise,
functionsInstance.cancelAllRequests
]);
Expand Down
6 changes: 1 addition & 5 deletions packages/functions/test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { FirebaseAuthInternalName } from '@firebase/auth-interop-types';
import { AppCheckInternalComponentName } from '@firebase/app-check-interop-types';
import { FunctionsService } from '../src/service';
import { connectFunctionsEmulator } from '../src/api';
import { fetch as undiciFetch } from 'undici';
import { MessagingInternalComponentName } from '../../../packages/messaging-interop-types';

export function makeFakeApp(options: FirebaseOptions = {}): FirebaseApp {
Expand Down Expand Up @@ -58,15 +57,12 @@ export function createTestService(
new ComponentContainer('test')
)
): FunctionsService {
const fetchImpl: typeof fetch =
typeof window !== 'undefined' ? fetch.bind(window) : (undiciFetch as any);
const functions = new FunctionsService(
app,
authProvider,
messagingProvider,
appCheckProvider,
region,
fetchImpl
region
);
const useEmulator = !!process.env.FIREBASE_FUNCTIONS_EMULATOR_ORIGIN;
if (useEmulator) {
Expand Down
4 changes: 0 additions & 4 deletions packages/rules-unit-testing/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,5 @@
"typings": "dist/index.d.ts",
"bugs": {
"url": "https://github.com/firebase/firebase-js-sdk/issues"
},
"dependencies": {
"node-fetch": "2.6.7",
"@types/node-fetch": "2.6.4"
}
}
5 changes: 2 additions & 3 deletions packages/rules-unit-testing/src/impl/discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/

import { EmulatorConfig, HostAndPort } from '../public_types';
import nodeFetch from 'node-fetch';
import { makeUrl, fixHostname } from './url';

/**
Expand All @@ -27,9 +26,9 @@ import { makeUrl, fixHostname } from './url';
*/
export async function discoverEmulators(
hub: HostAndPort,
fetch: typeof nodeFetch = nodeFetch
fetchImpl: typeof fetch = fetch
): Promise<DiscoveredEmulators> {
const res = await fetch(makeUrl(hub, '/emulators'));
const res = await fetchImpl(makeUrl(hub, '/emulators'));
if (!res.ok) {
throw new Error(
`HTTP Error ${res.status} when attempting to reach Emulator Hub at ${res.url}, are you sure it is running?`
Expand Down
1 change: 0 additions & 1 deletion packages/rules-unit-testing/src/impl/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import { HostAndPort } from '../public_types';
import { makeUrl } from './url';
import fetch from 'node-fetch';

/**
* @private
Expand Down
Loading
Loading