From eb1862b4e68b399eecc7267ea9e0bee36983b0cb Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Tue, 6 Sep 2022 19:04:56 +0200 Subject: [PATCH] Improve third-party Astro package support (#4623) --- .changeset/fifty-avocados-lay.md | 5 + packages/astro/src/core/create-vite.ts | 140 +++++++++++++++--- .../third-party-astro/astro.config.mjs | 4 + .../fixtures/third-party-astro/package.json | 9 ++ .../src/pages/astro-embed.astro | 16 ++ packages/astro/test/third-party-astro.test.js | 43 ++++++ pnpm-lock.yaml | 74 +++++++++ 7 files changed, 267 insertions(+), 24 deletions(-) create mode 100644 .changeset/fifty-avocados-lay.md create mode 100644 packages/astro/test/fixtures/third-party-astro/astro.config.mjs create mode 100644 packages/astro/test/fixtures/third-party-astro/package.json create mode 100644 packages/astro/test/fixtures/third-party-astro/src/pages/astro-embed.astro create mode 100644 packages/astro/test/third-party-astro.test.js diff --git a/.changeset/fifty-avocados-lay.md b/.changeset/fifty-avocados-lay.md new file mode 100644 index 000000000000..eaf9f2e32f5c --- /dev/null +++ b/.changeset/fifty-avocados-lay.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Improve third-party Astro package support diff --git a/packages/astro/src/core/create-vite.ts b/packages/astro/src/core/create-vite.ts index 81ac437210eb..e93fd7fb994e 100644 --- a/packages/astro/src/core/create-vite.ts +++ b/packages/astro/src/core/create-vite.ts @@ -2,6 +2,8 @@ import type { AstroConfig } from '../@types/astro'; import type { LogOptions } from './logger/core'; import fs from 'fs'; +import { createRequire } from 'module'; +import path from 'path'; import { fileURLToPath } from 'url'; import * as vite from 'vite'; import astroPostprocessVitePlugin from '../vite-plugin-astro-postprocess/index.js'; @@ -174,34 +176,115 @@ function sortPlugins(pluginOptions: vite.PluginOption[]) { // Scans `projectRoot` for third-party Astro packages that could export an `.astro` file // `.astro` files need to be built by Vite, so these should use `noExternal` async function getAstroPackages({ root }: AstroConfig): Promise { - const pkgUrl = new URL('./package.json', root); - const pkgPath = fileURLToPath(pkgUrl); - if (!fs.existsSync(pkgPath)) return []; + const { astroPackages } = new DependencyWalker(root); + return astroPackages; +} - const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); +/** + * Recursively walk a project’s dependency tree trying to find Astro packages. + * - If the current node is an Astro package, we continue walking its child dependencies. + * - If the current node is not an Astro package, we bail out of walking that branch. + * This assumes it is unlikely for Astro packages to be dependencies of packages that aren’t + * themselves also Astro packages. + */ +class DependencyWalker { + private readonly require: NodeRequire; + private readonly astroDeps = new Set(); + private readonly nonAstroDeps = new Set(); - const deps = [...Object.keys(pkg.dependencies || {}), ...Object.keys(pkg.devDependencies || {})]; + constructor(root: URL) { + const pkgUrl = new URL('./package.json', root); + this.require = createRequire(pkgUrl); + const pkgPath = fileURLToPath(pkgUrl); + if (!fs.existsSync(pkgPath)) return; - return deps.filter((dep) => { - // Attempt: package is common and not Astro. ❌ Skip these for perf - if (isCommonNotAstro(dep)) return false; - // Attempt: package is named `astro-something`. ✅ Likely a community package - if (/^astro\-/.test(dep)) return true; - const depPkgUrl = new URL(`./node_modules/${dep}/package.json`, root); - const depPkgPath = fileURLToPath(depPkgUrl); - if (!fs.existsSync(depPkgPath)) return false; + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); + const deps = [ + ...Object.keys(pkg.dependencies || {}), + ...Object.keys(pkg.devDependencies || {}), + ]; - const { - dependencies = {}, - peerDependencies = {}, - keywords = [], - } = JSON.parse(fs.readFileSync(depPkgPath, 'utf-8')); - // Attempt: package relies on `astro`. ✅ Definitely an Astro package - if (peerDependencies.astro || dependencies.astro) return true; - // Attempt: package is tagged with `astro` or `astro-component`. ✅ Likely a community package - if (keywords.includes('astro') || keywords.includes('astro-component')) return true; - return false; - }); + this.scanDependencies(deps); + } + + /** The dependencies we determined were likely to include `.astro` files. */ + public get astroPackages(): string[] { + return Array.from(this.astroDeps); + } + + private seen(dep: string): boolean { + return this.astroDeps.has(dep) || this.nonAstroDeps.has(dep); + } + + /** Try to load a directory’s `package.json` file from the filesystem. */ + private readPkgJSON(dir: string): PkgJSON | void { + try { + const filePath = path.join(dir, 'package.json'); + return JSON.parse(fs.readFileSync(filePath, 'utf-8')); + } catch (e) {} + } + + /** Try to resolve a dependency’s `package.json` even if not a package export. */ + private resolvePkgJSON(dep: string): PkgJSON | void { + try { + const pkgJson: PkgJSON = this.require(dep + '/package.json'); + return pkgJson; + } catch (e) { + // Most likely error is that the dependency doesn’t include `package.json` in its package `exports`. + try { + // Walk up from default export until we find `package.json` with name === dep. + let dir = path.dirname(this.require.resolve(dep)); + while (dir) { + const pkgJSON = this.readPkgJSON(dir); + if (pkgJSON && pkgJSON.name === dep) return pkgJSON; + + const parentDir = path.dirname(dir); + if (parentDir === dir) break; + + dir = parentDir; + } + } catch (e) { + // Give up! Who knows where the `package.json` is… + } + } + } + + private scanDependencies(deps: string[]): void { + const newDeps: string[] = []; + for (const dep of deps) { + // Attempt: package is common and not Astro. ❌ Skip these for perf + if (isCommonNotAstro(dep)) { + this.nonAstroDeps.add(dep); + continue; + } + + const pkgJson = this.resolvePkgJSON(dep); + if (!pkgJson) { + this.nonAstroDeps.add(dep); + continue; + } + const { dependencies = {}, peerDependencies = {}, keywords = [] } = pkgJson; + + if ( + // Attempt: package relies on `astro`. ✅ Definitely an Astro package + peerDependencies.astro || + dependencies.astro || + // Attempt: package is tagged with `astro` or `astro-component`. ✅ Likely a community package + keywords.includes('astro') || + keywords.includes('astro-component') || + // Attempt: package is named `astro-something` or `@scope/astro-something`. ✅ Likely a community package + /^(@[^\/]+\/)?astro\-/.test(dep) + ) { + this.astroDeps.add(dep); + // Collect any dependencies of this Astro package we haven’t seen yet. + const unknownDependencies = Object.keys(dependencies).filter((d) => !this.seen(d)); + newDeps.push(...unknownDependencies); + } else { + this.nonAstroDeps.add(dep); + } + } + if (newDeps.length) this.scanDependencies(newDeps); + } } const COMMON_DEPENDENCIES_NOT_ASTRO = [ @@ -256,3 +339,12 @@ function isCommonNotAstro(dep: string): boolean { ) ); } + +interface PkgJSON { + name: string; + dependencies?: Record; + devDependencies?: Record; + peerDependencies?: Record; + keywords?: string[]; + [key: string]: any; +} diff --git a/packages/astro/test/fixtures/third-party-astro/astro.config.mjs b/packages/astro/test/fixtures/third-party-astro/astro.config.mjs new file mode 100644 index 000000000000..882e6515a67e --- /dev/null +++ b/packages/astro/test/fixtures/third-party-astro/astro.config.mjs @@ -0,0 +1,4 @@ +import { defineConfig } from 'astro/config'; + +// https://astro.build/config +export default defineConfig({}); diff --git a/packages/astro/test/fixtures/third-party-astro/package.json b/packages/astro/test/fixtures/third-party-astro/package.json new file mode 100644 index 000000000000..26e11aefdd6f --- /dev/null +++ b/packages/astro/test/fixtures/third-party-astro/package.json @@ -0,0 +1,9 @@ +{ + "name": "@e2e/third-party-astro", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*", + "astro-embed": "^0.1.1" + } +} diff --git a/packages/astro/test/fixtures/third-party-astro/src/pages/astro-embed.astro b/packages/astro/test/fixtures/third-party-astro/src/pages/astro-embed.astro new file mode 100644 index 000000000000..a6deca343aa7 --- /dev/null +++ b/packages/astro/test/fixtures/third-party-astro/src/pages/astro-embed.astro @@ -0,0 +1,16 @@ +--- +import { YouTube } from 'astro-embed' +--- + + + + + Third-Party Package Test + + +
+

Third-Party .astro test

+ +
+ + diff --git a/packages/astro/test/third-party-astro.test.js b/packages/astro/test/third-party-astro.test.js new file mode 100644 index 000000000000..00d9e1f69979 --- /dev/null +++ b/packages/astro/test/third-party-astro.test.js @@ -0,0 +1,43 @@ +import { expect } from 'chai'; +import * as cheerio from 'cheerio'; +import { loadFixture } from './test-utils.js'; + +describe('third-party .astro component', () => { + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/third-party-astro/', + }); + }); + + describe('build', () => { + before(async () => { + await fixture.build(); + }); + + it('renders a page using a third-party .astro component', async () => { + const html = await fixture.readFile('/astro-embed/index.html'); + const $ = cheerio.load(html); + expect($('h1').text()).to.equal('Third-Party .astro test'); + }); + }); + + describe('dev', () => { + let devServer; + + before(async () => { + devServer = await fixture.startDevServer(); + }); + + after(async () => { + await devServer.stop(); + }); + + it('renders a page using a third-party .astro component', async () => { + const html = await fixture.fetch('/astro-embed/').then((res) => res.text()); + const $ = cheerio.load(html); + expect($('h1').text()).to.equal('Third-Party .astro test'); + }); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index df76eaa373d7..a48d94cd43ab 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -972,6 +972,14 @@ importers: '@astrojs/tailwind': link:../../../../integrations/tailwind astro: link:../../.. + packages/astro/e2e/fixtures/third-party-astro: + specifiers: + astro: workspace:* + astro-embed: ^0.1.1 + dependencies: + astro: link:../../.. + astro-embed: 0.1.1_astro@packages+astro + packages/astro/e2e/fixtures/ts-resolution: specifiers: '@astrojs/react': workspace:* @@ -2029,6 +2037,14 @@ importers: postcss: 8.4.16 tailwindcss: 3.1.8_postcss@8.4.16 + packages/astro/test/fixtures/third-party-astro: + specifiers: + astro: workspace:* + astro-embed: ^0.1.1 + dependencies: + astro: link:../../.. + astro-embed: 0.1.1_astro@packages+astro + packages/astro/test/fixtures/type-imports: specifiers: astro: workspace:* @@ -3174,6 +3190,39 @@ packages: leven: 3.1.0 dev: false + /@astro-community/astro-embed-integration/0.1.0_astro@packages+astro: + resolution: {integrity: sha512-qR4us0hAqIYo6MduvpXLrjeakX04afDILa7WkQbmWj3c4sbOqIcFCirLrmFs+dPjcPkv2Zpl2l3PxN3G6+ONSA==} + peerDependencies: + astro: ^1.0.0-beta.10 + dependencies: + '@astro-community/astro-embed-twitter': 0.1.3_astro@packages+astro + '@astro-community/astro-embed-youtube': 0.1.1_astro@packages+astro + astro: link:packages/astro + unist-util-select: 4.0.1 + dev: false + + /@astro-community/astro-embed-twitter/0.1.3_astro@packages+astro: + resolution: {integrity: sha512-lcOBnzhczNrngkafzD+8BGKiK8oJvahg3/QUuWgueNwHRU8C+18brdxKc1i4ttZWgAt1A5u+jx21Tc4bquMUzg==} + peerDependencies: + astro: ^1.0.0-beta.10 + dependencies: + '@astro-community/astro-embed-utils': 0.0.3 + astro: link:packages/astro + dev: false + + /@astro-community/astro-embed-utils/0.0.3: + resolution: {integrity: sha512-hXwSMtSAL3V9fnFHps+/CoDIJst26U/qSdI7srIQ8GPmFqdbcqJd/qOqYzGezAR/qTM8gmTjDCGOuVI0Z+xT3Q==} + dev: false + + /@astro-community/astro-embed-youtube/0.1.1_astro@packages+astro: + resolution: {integrity: sha512-qIf5nr3BMB/pWJWf7x/t86CIjpPA69eVKQql7TNJW7lTYL2SVPFA9WowsfvvrhNN9aWV/kTaSpW9e/m4FtXdkQ==} + peerDependencies: + astro: ^1.0.0-beta.10 + dependencies: + astro: link:packages/astro + lite-youtube-embed: 0.2.0 + dev: false + /@astrojs/compiler/0.19.0: resolution: {integrity: sha512-8nvyxZTfCXLyRmYfTttpJT6EPhfBRg0/q4J/Jj3/pNPLzp+vs05ZdktsY6QxAREaOMAnNEtSqcrB4S5DsXOfRg==} dev: true @@ -9727,6 +9776,17 @@ packages: hasBin: true dev: false + /astro-embed/0.1.1_astro@packages+astro: + resolution: {integrity: sha512-NBnLDB0PygbahCBFeGDPzmW4/PJSrieWgjN7mN8vmStUACM+cdTz1vhLDSWt4LlbWxozq0x9G1dTnoVbHyYKLA==} + peerDependencies: + astro: ^1.0.0-beta.10 + dependencies: + '@astro-community/astro-embed-integration': 0.1.0_astro@packages+astro + '@astro-community/astro-embed-twitter': 0.1.3_astro@packages+astro + '@astro-community/astro-embed-youtube': 0.1.1_astro@packages+astro + astro: link:packages/astro + dev: false + /async/3.2.4: resolution: {integrity: sha512-iAB+JbDEGXhyIUavoDl9WP/Jj106Kz9DEn1DPgYw5ruDn0e3Wgi3sKFm55sASdGBNOQB8F59d9qQ7deqrHA8wQ==} dev: false @@ -13056,6 +13116,10 @@ packages: lit-html: 2.3.1 dev: false + /lite-youtube-embed/0.2.0: + resolution: {integrity: sha512-XXXAk5sbvtjjwbie3XG+6HppgTm1HTGL/Uk9z9NkJH53o7puZLur434heHzAjkS60hZB3vT4ls25zl5rMiX4EA==} + dev: false + /load-yaml-file/0.2.0: resolution: {integrity: sha512-OfCBkGEw4nN6JLtgRidPX6QxjBQGQf72q3si2uvqyFEMbycSFFHwAZeXx6cJgFM9wmLrf9zBwCP3Ivqa+LLZPw==} engines: {node: '>=6'} @@ -17016,6 +17080,16 @@ packages: unist-util-visit: 4.1.1 dev: false + /unist-util-select/4.0.1: + resolution: {integrity: sha512-zPozyEo5vr1csbHf1TqlQrnuLVJ0tNMo63og3HrnINh2+OIDAgQpqHVr+0BMw1DIVHJV8ft/e6BZqtvD1Y5enw==} + dependencies: + '@types/unist': 2.0.6 + css-selector-parser: 1.4.1 + nth-check: 2.1.1 + unist-util-is: 5.1.1 + zwitch: 2.0.2 + dev: false + /unist-util-stringify-position/3.0.2: resolution: {integrity: sha512-7A6eiDCs9UtjcwZOcCpM4aPII3bAAGv13E96IkawkOAW0OhH+yRxtY0lzo8KiHpzEMfH7Q+FizUmwp8Iqy5EWg==} dependencies: