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

refactor: stop using node-fetch inside create-remix #7345

Merged
merged 6 commits into from
Dec 8, 2023
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
5 changes: 5 additions & 0 deletions .changeset/create-remix-fetch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"create-remix": patch
---

Switch to using `@remix-run/web-fetch` instead of `node-fetch` inside the `create-remix` CLI
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"@octokit/plugin-paginate-rest": "^2.17.0",
"@octokit/rest": "^18.12.0",
"@remix-run/changelog-github": "^0.0.5",
"@remix-run/web-fetch": "^4.4.0-pre.0",
"@rollup/plugin-babel": "^5.2.2",
"@rollup/plugin-json": "^5.0.0",
"@rollup/plugin-node-resolve": "^11.0.1",
Expand Down
50 changes: 41 additions & 9 deletions packages/create-remix/copy-template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import fs from "node:fs";
import path from "node:path";
import stream from "node:stream";
import { promisify } from "node:util";
import fetch from "node-fetch";
import { fetch } from "@remix-run/web-fetch";
import gunzip from "gunzip-maybe";
import tar from "tar-fs";
import { ProxyAgent } from "proxy-agent";
Expand All @@ -28,9 +28,9 @@ export async function copyTemplate(
/**
* Valid templates are:
* - local file or directory on disk
* - github owner/repo shorthand
* - github owner/repo/directory shorthand
* - full github repo URL
* - GitHub owner/repo shorthand
* - GitHub owner/repo/directory shorthand
* - full GitHub repo URL
* - any tarball URL
*/

Expand Down Expand Up @@ -140,7 +140,7 @@ async function copyTemplateFromLocalFilePath(
return false;
}
if (fs.statSync(filePath).isDirectory()) {
// If our template is just a directory on disk, return true here and we'll
// If our template is just a directory on disk, return true here, and we'll
// just copy directly from there instead of "extracting" to a temp
// directory first
return true;
Expand Down Expand Up @@ -224,7 +224,7 @@ async function downloadAndExtractTarball(
headers.Authorization = `token ${token}`;
}
if (isGithubReleaseAssetUrl(tarballUrl)) {
// We can download the asset via the github api, but first we need to look
// We can download the asset via the GitHub api, but first we need to look
// up the asset id
let info = getGithubReleaseAssetInfo(tarballUrl);
headers.Accept = "application/vnd.github.v3+json";
Expand Down Expand Up @@ -297,16 +297,20 @@ async function downloadAndExtractTarball(
);
}

// file paths returned from github are always unix style
// file paths returned from GitHub are always unix style
if (filePath) {
filePath = filePath.split(path.sep).join(path.posix.sep);
}

let filePathHasFiles = false;

try {
let input = new stream.PassThrough();
// Start reading stream into passthrough, don't await to avoid buffering
writeReadableStreamToWritable(response.body, input);
await pipeline(
response.body.pipe(gunzip()),
input,
gunzip(),
tar.extract(downloadPath, {
map(header) {
let originalDirName = header.name.split("/")[0];
Expand Down Expand Up @@ -356,6 +360,34 @@ async function downloadAndExtractTarball(
}
}

// Copied from remix-node/stream.ts
async function writeReadableStreamToWritable(
stream: ReadableStream,
writable: stream.Writable
) {
let reader = stream.getReader();
let flushable = writable as { flush?: Function };

try {
while (true) {
let { done, value } = await reader.read();

if (done) {
writable.end();
break;
}

writable.write(value);
if (typeof flushable.flush === "function") {
flushable.flush();
}
}
} catch (error: unknown) {
writable.destroy(error as Error);
throw error;
}
}

function isValidGithubRepoUrl(
input: string | URL
): input is URL | GithubUrlString {
Expand Down Expand Up @@ -419,7 +451,7 @@ function getGithubReleaseAssetInfo(browserUrl: string): ReleaseAssetInfo {
let [, owner, name, , downloadOrLatest, tag, asset] = url.pathname.split("/");

if (downloadOrLatest === "latest" && tag === "download") {
// handle the Github URL quirk for latest releases
// handle the GitHub URL quirk for latest releases
tag = "latest";
}

Expand Down
3 changes: 1 addition & 2 deletions packages/create-remix/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
"create-remix": "dist/cli.js"
},
"dependencies": {
"@remix-run/web-fetch": "^4.4.2",
"arg": "^5.0.1",
"chalk": "^4.1.2",
"execa": "5.1.1",
"gunzip-maybe": "^1.4.2",
"log-update": "^5.0.1",
"node-fetch": "^2.6.9",
"proxy-agent": "^6.3.0",
"recursive-readdir": "^2.2.3",
"rimraf": "^4.1.2",
Expand All @@ -34,7 +34,6 @@
},
"devDependencies": {
"@types/gunzip-maybe": "^1.4.0",
"@types/node-fetch": "^2.5.7",
"@types/recursive-readdir": "^2.2.1",
"@types/tar-fs": "^2.0.1",
"esbuild": "0.17.6",
Expand Down
2 changes: 0 additions & 2 deletions packages/remix-dev/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
"lodash": "^4.17.21",
"lodash.debounce": "^4.0.8",
"minimatch": "^9.0.0",
"node-fetch": "^2.6.9",
"ora": "^5.4.1",
"picocolors": "^1.0.0",
"picomatch": "^2.3.1",
Expand Down Expand Up @@ -81,7 +80,6 @@
"@types/jsesc": "^3.0.1",
"@types/lodash.debounce": "^4.0.6",
"@types/node": "^18.17.1",
"@types/node-fetch": "^2.5.7",
"@types/npmcli__package-json": "^4.0.0",
"@types/picomatch": "^2.3.0",
"@types/prettier": "^2.7.3",
Expand Down
2 changes: 1 addition & 1 deletion scripts/deployment-test/_shared.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import crypto from "node:crypto";
import { sync as spawnSync } from "cross-spawn";
import PackageJson from "@npmcli/package-json";
import jsonfile from "jsonfile";
import fetch from "node-fetch";
import { fetch } from "@remix-run/web-fetch";
import retry from "fetch-retry";

let fetchRetry = retry(fetch);
Expand Down
2 changes: 1 addition & 1 deletion scripts/deployment-test/cf-pages.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import path from "node:path";
import { sync as spawnSync } from "cross-spawn";
import fse from "fs-extra";
import fetch from "node-fetch";
import { fetch } from "@remix-run/web-fetch";
import PackageJson from "@npmcli/package-json";

import {
Expand Down
2 changes: 1 addition & 1 deletion scripts/deployment-test/cf-workers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import path from "node:path";
import { sync as spawnSync } from "cross-spawn";
import fse from "fs-extra";
import toml from "@iarna/toml";
import fetch from "node-fetch";
import { fetch } from "@remix-run/web-fetch";
import PackageJson from "@npmcli/package-json";

import {
Expand Down
27 changes: 5 additions & 22 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2327,10 +2327,10 @@
"@remix-run/web-stream" "^1.1.0"
web-encoding "1.1.5"

"@remix-run/web-fetch@^4.4.1":
version "4.4.1"
resolved "https://registry.npmjs.org/@remix-run/web-fetch/-/web-fetch-4.4.1.tgz#1ea34e6f1c660a52e7582007917a552f0efdc58b"
integrity sha512-xMceEGn2kvfeWS91nHSOhEQHPGgjFnmDVpWFZrbWPVdiTByMZIn421/tdSF6Kd1RsNsY+5Iwt3JFEKZHAcMQHw==
"@remix-run/web-fetch@^4.4.0-pre.0", "@remix-run/web-fetch@^4.4.1", "@remix-run/web-fetch@^4.4.2":
version "4.4.2"
resolved "https://registry.npmjs.org/@remix-run/web-fetch/-/web-fetch-4.4.2.tgz#ce7aedef72cc26e15060e8cf84674029f92809b6"
integrity sha512-jgKfzA713/4kAW/oZ4bC3MoLWyjModOVDjFPNseVqcJKSafgIscrYL9G50SurEYLswPuoU3HzSbO0jQCMYWHhA==
dependencies:
"@remix-run/web-blob" "^3.1.0"
"@remix-run/web-file" "^3.1.0"
Expand Down Expand Up @@ -2967,14 +2967,6 @@
resolved "https://registry.npmjs.org/@types/ms/-/ms-0.7.31.tgz"
integrity sha512-iiUgKzV9AuaEkZqkOLDIvlQiL6ltuZd9tGcW3gwpnX8JbuiuhFlEGmmFXEXkN50Cvq7Os88IY2v0dkDqXYWVgA==

"@types/node-fetch@^2.5.7":
version "2.5.12"
resolved "https://registry.npmjs.org/@types/node-fetch/-/node-fetch-2.5.12.tgz"
integrity sha512-MKgC4dlq4kKNa/mYrwpKfzQMB5X3ee5U6fSprkKpToBqBmX4nFZL9cW5jl6sWn+xpRJ7ypWh2yyqqr8UUCstSw==
dependencies:
"@types/node" "*"
form-data "^3.0.0"

"@types/node@*", "@types/node@^18.17.1":
version "18.17.1"
resolved "https://registry.npmjs.org/@types/node/-/node-18.17.1.tgz#84c32903bf3a09f7878c391d31ff08f6fe7d8335"
Expand Down Expand Up @@ -6319,15 +6311,6 @@ forever-agent@~0.6.1:
resolved "https://registry.npmjs.org/forever-agent/-/forever-agent-0.6.1.tgz"
integrity sha1-+8cfDEGt6zf5bFd60e1C2P2sypE=

form-data@^3.0.0:
version "3.0.1"
resolved "https://registry.npmjs.org/form-data/-/form-data-3.0.1.tgz"
integrity sha512-RHkBKtLWUVwd7SqRIvCZMEvAMoGUp0XU+seQiZejj0COz3RI3hWP4sCv3gZWWLjJTd7rGwcsF5eKZGii0r/hbg==
dependencies:
asynckit "^0.4.0"
combined-stream "^1.0.8"
mime-types "^2.1.12"

form-data@^4.0.0:
version "4.0.0"
resolved "https://registry.npmjs.org/form-data/-/form-data-4.0.0.tgz#93919daeaf361ee529584b9b31664dc12c9fa452"
Expand Down Expand Up @@ -9905,7 +9888,7 @@ nice-try@^1.0.4:
resolved "https://registry.npmjs.org/nice-try/-/nice-try-1.0.5.tgz"
integrity sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ==

node-fetch@^2.5.0, node-fetch@^2.6.7, node-fetch@^2.6.9:
node-fetch@^2.5.0, node-fetch@^2.6.7:
version "2.6.9"
resolved "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.9.tgz#7c7f744b5cc6eb5fd404e0c7a9fec630a55657e6"
integrity sha512-DJm/CJkZkRjKKj4Zi4BsKVZh3ValV5IR5s7LVZnW+6YMh0W1BfNA8XSs6DLMGYlId5F3KnA70uu2qepcR08Qqg==
Expand Down
Loading