Make fetch injection safe to existing code (#2135)

* fix: inject `fetch` only if not declared * test: improve fetch test * chore(lint): Prettier fix * chore: trigger ci * fix: improve type narrowing Co-authored-by: GitHub Action <[email protected]>
withastro · Dec 6, 2021 · 77c3fda · 77c3fda
1 parent 1298fd8
commit 77c3fda
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 0 deletions.
diff --git a/.changeset/wet-insects-dance.md b/.changeset/wet-insects-dance.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Patch `fetch` support to prioritize authored code. Existing `fetch` imports and declarations are respected.
diff --git a/packages/astro/package.json b/packages/astro/package.json
@@ -76,6 +76,7 @@
  "es-module-lexer": "^0.7.1",
  "esbuild": "0.13.7",
  "estree-util-value-to-estree": "^1.2.0",
+ "estree-walker": "^3.0.0",
  "fast-glob": "^3.2.7",
  "fast-xml-parser": "^3.19.0",
  "html-entities": "^2.3.2",

diff --git a/packages/astro/src/vite-plugin-fetch/index.ts b/packages/astro/src/vite-plugin-fetch/index.ts
@@ -1,5 +1,7 @@
 import type { Plugin } from '../core/vite';
+import type { BaseNode, Identifier } from 'estree';
 import MagicString from 'magic-string';
+import { walk } from 'estree-walker';
 
 // https://github.com/vitejs/vite/discussions/5109#discussioncomment-1450726
 function isSSR(options: undefined | boolean | { ssr: boolean }): boolean {
@@ -21,6 +23,10 @@ const SUPPORTED_FILES = /\.(astro|svelte|vue|[cm]?js|jsx|[cm]?ts|tsx)$/;
 const IGNORED_MODULES = [/astro\/dist\/runtime\/server/, /\/node-fetch\//];
 const DEFINE_FETCH = `import fetch from 'node-fetch';\n`;
 
+function isIdentifier(node: BaseNode): node is Identifier {
+ return node.type === 'Identifier';
+}
+
 export default function pluginFetch(): Plugin {
  return {
  name: '@astrojs/vite-plugin-fetch',
@@ -39,6 +45,26 @@ export default function pluginFetch(): Plugin {
  if (!code.includes('fetch')) {
  return null;
  }
+
+ const ast = this.parse(code);
+ let fetchDeclared = false;
+ walk(ast, {
+ enter(node, parent) {
+ if (fetchDeclared) return this.skip();
+ if (isIdentifier(node)) {
+ // Identifier is OK in any type of Expression (CallExpression, UnaryExpression, etc)
+ if (node.name === 'fetch' && !parent.type.endsWith('Expression')) {
+ fetchDeclared = true;
+ }
+ }
+ },
+ });
+
+ // Fetch is already declared, do not inject a re-declaration!
+ if (fetchDeclared) {
+ return null;
+ }
+
  // Ignore specific modules
  for (const ignored of IGNORED_MODULES) {
  if (id.match(ignored)) {

diff --git a/packages/astro/test/fetch.test.js b/packages/astro/test/fetch.test.js
@@ -27,4 +27,10 @@ describe('Global Fetch', () => {
  expect($('#svelte').text()).to.equal('function', 'Fetch supported in .svelte');
  expect($('#vue').text()).to.equal('function', 'Fetch supported in .vue');
  });
+ it('Respects existing code', async () => {
+ const html = await fixture.readFile('/index.html');
+ const $ = cheerio.load(html);
+ expect($('#already-imported').text()).to.equal('function', 'Existing fetch imports respected');
+ expect($('#custom-declaration').text()).to.equal('number', 'Custom fetch declarations respected');
+ });
 });
diff --git a/packages/astro/test/fixtures/fetch/src/components/AlreadyImported.astro b/packages/astro/test/fixtures/fetch/src/components/AlreadyImported.astro
@@ -0,0 +1,5 @@
+---
+import fetch from 'node-fetch'
+---
+
+<span id="already-imported">{typeof fetch}</span>
diff --git a/packages/astro/test/fixtures/fetch/src/components/CustomDeclaration.astro b/packages/astro/test/fixtures/fetch/src/components/CustomDeclaration.astro
@@ -0,0 +1,5 @@
+---
+const fetch = 0;
+---
+
+<span id="custom-declaration">{typeof fetch}</span>
diff --git a/packages/astro/test/fixtures/fetch/src/pages/index.astro b/packages/astro/test/fixtures/fetch/src/pages/index.astro
@@ -1,5 +1,7 @@
 ---
 import Test from '../components/AstroComponent.astro';
+import AlreadyImported from '../components/AlreadyImported.astro';
+import CustomDeclaration from '../components/CustomDeclaration.astro';
 import JsxComponent from '../components/JsxComponent.jsx';
 import SvelteComponent from '../components/SvelteComponent.svelte';
 import VueComponent from '../components/VueComponent.vue';
@@ -12,6 +14,8 @@ import VueComponent from '../components/VueComponent.vue';
 <body>
  <span id="astro-page">{typeof fetch}</span>
  <Test />
+ <AlreadyImported />
+ <CustomDeclaration />
  <JsxComponent />
  <SvelteComponent />
  <VueComponent />

diff --git a/yarn.lock b/yarn.lock
@@ -5213,6 +5213,11 @@ estree-walker@^2.0.1, estree-walker@^2.0.2:
  resolved "https://registry.yarnpkg.com/estree-walker/-/estree-walker-2.0.2.tgz#52f010178c2a4c117a7757cfe942adb7d2da4cac"
  integrity sha512-Rfkk/Mp/DL7JVje3u18FxFujQlTNR2q6QfMSMB7AvCBx91NGj/ba3kCfza0f6dVDbw7YlRf/nDrn7pQrCCyQ/w==
 
+estree-walker@^3.0.0:
+ version "3.0.0"
+ resolved "https://registry.yarnpkg.com/estree-walker/-/estree-walker-3.0.0.tgz#ca4b284de9dffb255288c76a44870b360faf14f9"
+ integrity sha512-s6ceX0NFiU/vKPiKvFdR83U1Zffu7upwZsGwpoqfg5rbbq1l50WQ5hCeIvM6E6oD4shUHCYMsiFPns4Jk0YfMQ==
+
 esutils@^2.0.2:
  version "2.0.3"
  resolved "https://registry.yarnpkg.com/esutils/-/esutils-2.0.3.tgz#74d2eb4de0b8da1293711910d50775b9b710ef64"