Skip to content

Commit

Permalink
chore: remove undici polyfill (#8729)
Browse files Browse the repository at this point in the history
  • Loading branch information
lilnasy committed Oct 4, 2023
1 parent 272ad45 commit 21e0757
Show file tree
Hide file tree
Showing 12 changed files with 22 additions and 105 deletions.
5 changes: 5 additions & 0 deletions .changeset/green-crabs-breathe.md
@@ -0,0 +1,5 @@
---
'@astrojs/telemetry': patch
---

Removed an unnecessary dependency.
5 changes: 5 additions & 0 deletions .changeset/thirty-ravens-fly.md
@@ -0,0 +1,5 @@
---
'astro': patch
---

Node-based adapters now create less server-side javascript
7 changes: 1 addition & 6 deletions packages/astro/astro.js
Expand Up @@ -3,7 +3,6 @@

// ISOMORPHIC FILE: NO TOP-LEVEL IMPORT/REQUIRE() ALLOWED
// This file has to run as both ESM and CJS on older Node.js versions
// Needed for Stackblitz: https://github.com/stackblitz/webcontainer-core/issues/281

const CI_INSTRUCTIONS = {
NETLIFY: 'https://docs.netlify.com/configure-builds/manage-dependencies/#node-js-and-javascript',
Expand All @@ -16,15 +15,11 @@ const CI_INSTRUCTIONS = {
const engines = '>=18.14.1';
const skipSemverCheckIfAbove = 19;

// HACK (2023-08-18) Stackblitz does not support Node 18 yet, so we'll fake Node 16 support for some time until it's supported
// TODO: Remove when Node 18 is supported on Stackblitz
const isStackblitz = process.env.SHELL === '/bin/jsh' && process.versions.webcontainer != null;

/** `astro *` */
async function main() {
const version = process.versions.node;
// Fast-path for higher Node.js versions
if (!isStackblitz && (parseInt(version) || 0) <= skipSemverCheckIfAbove) {
if ((parseInt(version) || 0) <= skipSemverCheckIfAbove) {
try {
const semver = await import('semver');
if (!semver.satisfies(version, engines)) {
Expand Down
1 change: 0 additions & 1 deletion packages/astro/package.json
Expand Up @@ -168,7 +168,6 @@
"string-width": "^6.1.0",
"strip-ansi": "^7.1.0",
"tsconfig-resolver": "^3.0.1",
"undici": "^5.23.0",
"unist-util-visit": "^4.1.2",
"vfile": "^5.3.7",
"vite": "^4.4.9",
Expand Down
15 changes: 1 addition & 14 deletions packages/astro/src/core/endpoint/index.ts
Expand Up @@ -39,7 +39,6 @@ export function createAPIContext({
props,
adapterName,
}: CreateAPIContext): APIContext {
initResponseWithEncoding();
const context = {
cookies: new AstroCookies(request),
request,
Expand Down Expand Up @@ -92,10 +91,7 @@ export function createAPIContext({

type ResponseParameters = ConstructorParameters<typeof Response>;

export let ResponseWithEncoding: ReturnType<typeof initResponseWithEncoding>;
// TODO Remove this after StackBlitz supports Node 18.
let initResponseWithEncoding = () => {
class LocalResponseWithEncoding extends Response {
export class ResponseWithEncoding extends Response {
constructor(
body: ResponseParameters[0],
init: ResponseParameters[1],
Expand All @@ -122,15 +118,6 @@ let initResponseWithEncoding = () => {
}
}

// Set the module scoped variable.
ResponseWithEncoding = LocalResponseWithEncoding;

// Turn this into a noop.
initResponseWithEncoding = (() => {}) as any;

return LocalResponseWithEncoding;
};

export async function callEndpoint<MiddlewareResult = Response | EndpointOutput>(
mod: EndpointHandler,
env: Environment,
Expand Down
60 changes: 2 additions & 58 deletions packages/astro/src/core/polyfill.ts
@@ -1,63 +1,7 @@
import crypto from 'node:crypto';
import {
ByteLengthQueuingStrategy,
CountQueuingStrategy,
ReadableByteStreamController,
ReadableStream,
ReadableStreamBYOBReader,
ReadableStreamBYOBRequest,
ReadableStreamDefaultController,
ReadableStreamDefaultReader,
TransformStream,
WritableStream,
WritableStreamDefaultController,
WritableStreamDefaultWriter,
} from 'node:stream/web';
import { File, FormData, Headers, Request, Response, fetch } from 'undici';

// NOTE: This file does not intend to polyfill everything that exists, its main goal is to make life easier
// for users deploying to runtime that do support these features. In the future, we hope for this file to disappear.

// HACK (2023-08-18) Stackblitz does not support Node 18 yet, so we'll fake Node 16 support for some time until it's supported
// TODO: Remove when Node 18 is supported on Stackblitz. File should get imported from `node:buffer` instead of `undici` once this is removed
const isStackblitz = process.env.SHELL === '/bin/jsh' && process.versions.webcontainer != null;
import buffer from 'node:buffer';

export function apply() {
if (isStackblitz) {
const neededPolyfills = {
ByteLengthQueuingStrategy,
CountQueuingStrategy,
ReadableByteStreamController,
ReadableStream,
ReadableStreamBYOBReader,
ReadableStreamBYOBRequest,
ReadableStreamDefaultController,
ReadableStreamDefaultReader,
TransformStream,
WritableStream,
WritableStreamDefaultController,
WritableStreamDefaultWriter,
File,
FormData,
Headers,
Request,
Response,
fetch,
};

for (let polyfillName of Object.keys(neededPolyfills)) {
if (Object.hasOwnProperty.call(globalThis, polyfillName)) continue;

// Add polyfill to globalThis
Object.defineProperty(globalThis, polyfillName, {
configurable: true,
enumerable: true,
writable: true,
value: neededPolyfills[polyfillName as keyof typeof neededPolyfills],
});
}
}

// Remove when Node 18 is dropped for Node 20
if (!globalThis.crypto) {
Object.defineProperty(globalThis, 'crypto', {
Expand All @@ -68,7 +12,7 @@ export function apply() {
// Remove when Node 18 is dropped for Node 20
if (!globalThis.File) {
Object.defineProperty(globalThis, 'File', {
value: File,
value: buffer.File,
});
}
}
2 changes: 1 addition & 1 deletion packages/astro/test/test-utils.js
Expand Up @@ -170,7 +170,7 @@ export async function loadFixture(inlineConfig) {
try {
return await fetch(resolvedUrl, init);
} catch (err) {
// undici throws a vague error when it fails, so we log the url here to easily debug it
// node fetch throws a vague error when it fails, so we log the url here to easily debug it
if (err.message?.includes('fetch failed')) {
console.error(`[astro test] failed to fetch ${resolvedUrl}`);
console.error(err);
Expand Down
3 changes: 1 addition & 2 deletions packages/integrations/node/package.json
Expand Up @@ -49,8 +49,7 @@
"cheerio": "1.0.0-rc.12",
"express": "^4.18.2",
"mocha": "^10.2.0",
"node-mocks-http": "^1.13.0",
"undici": "^5.23.0"
"node-mocks-http": "^1.13.0"
},
"publishConfig": {
"provenance": true
Expand Down
17 changes: 5 additions & 12 deletions packages/integrations/node/src/createOutgoingHttpHeaders.ts
Expand Up @@ -8,15 +8,12 @@ import type { OutgoingHttpHeaders } from 'node:http';
* @returns NodeJS OutgoingHttpHeaders object with multiple set-cookie handled as an array of values
*/
export const createOutgoingHttpHeaders = (
webHeaders: Headers | undefined | null
headers: Headers | undefined | null
): OutgoingHttpHeaders | undefined => {
if (!webHeaders) {
if (!headers) {
return undefined;
}

// re-type to access Header.getSetCookie()
const headers = webHeaders as HeadersWithGetSetCookie;


// at this point, a multi-value'd set-cookie header is invalid (it was concatenated as a single CSV, which is not valid for set-cookie)
const nodeHeaders: OutgoingHttpHeaders = Object.fromEntries(headers.entries());

Expand All @@ -26,7 +23,8 @@ export const createOutgoingHttpHeaders = (

// if there is > 1 set-cookie header, we have to fix it to be an array of values
if (headers.has('set-cookie')) {
const cookieHeaders = headers.getSetCookie();
// @ts-expect-error
const cookieHeaders = headers.getSetCookie() as string[];
if (cookieHeaders.length > 1) {
// the Headers.entries() API already normalized all header names to lower case so we can safely index this as 'set-cookie'
nodeHeaders['set-cookie'] = cookieHeaders;
Expand All @@ -35,8 +33,3 @@ export const createOutgoingHttpHeaders = (

return nodeHeaders;
};

interface HeadersWithGetSetCookie extends Headers {
// the @astrojs/webapi polyfill makes this available (as of undici@5.19.0), but tsc doesn't pick it up on the built-in Headers type from DOM lib
getSetCookie(): string[];
}
1 change: 0 additions & 1 deletion packages/telemetry/package.json
Expand Up @@ -35,7 +35,6 @@
"dset": "^3.1.2",
"is-docker": "^3.0.0",
"is-wsl": "^3.0.0",
"undici": "^5.23.0",
"which-pm-runs": "^1.1.0"
},
"devDependencies": {
Expand Down
1 change: 0 additions & 1 deletion packages/telemetry/src/post.ts
@@ -1,5 +1,4 @@
const ASTRO_TELEMETRY_ENDPOINT = `https://telemetry.astro.build/api/v1/record`;
import { fetch } from 'undici';

export function post(body: Record<string, any>): Promise<any> {
return fetch(ASTRO_TELEMETRY_ENDPOINT, {
Expand Down
10 changes: 1 addition & 9 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 21e0757

Please sign in to comment.