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

Remove shamefully-hoist #4842

Merged
merged 28 commits into from
Sep 28, 2022
Merged
Show file tree
Hide file tree
Changes from 27 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/itchy-guests-perform.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/image': patch
---

Specify sharp as optional peer dependency
5 changes: 5 additions & 0 deletions .changeset/odd-elephants-argue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/markdown-remark': patch
---

Fix non-hoisted remark/rehype plugin loading
15 changes: 15 additions & 0 deletions .changeset/silver-rats-know.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'astro': patch
'@astrojs/image': patch
'@astrojs/mdx': patch
'@astrojs/netlify': patch
'@astrojs/preact': patch
'@astrojs/rss': patch
'@astrojs/svelte': patch
'@astrojs/tailwind': patch
'@astrojs/vue': patch
'@astrojs/markdown-remark': patch
'@astrojs/telemetry': patch
---

Add missing dependencies, support strict dependency installation (e.g. pnpm)
5 changes: 5 additions & 0 deletions .changeset/young-pens-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/rss': patch
---

Remove path-browserify dependency
3 changes: 2 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,9 @@ jobs:
repository: withastro/docs
path: smoke/docs

# NOTE: remove shamefully-hoist when docs is also not hoisted
- name: Install dependencies
run: pnpm install --no-frozen-lockfile
run: pnpm install --no-frozen-lockfile --shamefully-hoist

- name: Build Packages
run: pnpm run build
Expand Down
26 changes: 10 additions & 16 deletions .npmrc
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,13 @@ link-workspace-packages=true
save-workspace-protocol=false # This prevents the examples to have the `workspace:` prefix
auto-install-peers=false

shamefully-hoist=true
# TODO: We would like to move to individual opt-in hoisting, but Astro was not originally
# written with this in mind. In the future, it would be good to hoist individual packages only.
# public-hoist-pattern[]=autoprefixer
# public-hoist-pattern[]=astro
# public-hoist-pattern[]=remark-*
# public-hoist-pattern[]=rehype-*
# public-hoist-pattern[]=react
# public-hoist-pattern[]=react-dom
# public-hoist-pattern[]=preact
# public-hoist-pattern[]=preact-render-to-string
# public-hoist-pattern[]=vue
# public-hoist-pattern[]=svelte
# public-hoist-pattern[]=solid-js
# public-hoist-pattern[]=lit
# public-hoist-pattern[]=@webcomponents/template-shadowroot
# `github-slugger` is used by `vite-plugin-markdown-legacy`.
# Temporarily hoist this until we remove the feature.
public-hoist-pattern[]=github-slugger
# Vite's esbuild optimizer has trouble optimizing `@astrojs/lit/client-shim.js`
# which imports this dependency.
public-hoist-pattern[]=@webcomponents/template-shadowroot
# On Windows, `svelte-preprocess` can't import `svelte/compiler`. Might be a pnpm bug.
public-hoist-pattern[]=svelte
# There's a lit dependency duplication somewhere causing multiple Lit versions error.
public-hoist-pattern[]=*lit*
2 changes: 0 additions & 2 deletions examples/basics/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/blog/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/deno/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/docs/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/framework-alpine/.npmrc

This file was deleted.

3 changes: 1 addition & 2 deletions examples/framework-lit/.npmrc
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
# Expose Astro dependencies for `pnpm` users
shamefully-hoist=true
public-hoist-pattern[]=*lit*
2 changes: 0 additions & 2 deletions examples/framework-multiple/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/framework-preact/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/framework-react/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/framework-solid/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/framework-svelte/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/framework-vue/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/minimal/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/non-html-pages/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/portfolio/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/ssr/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/with-markdown-plugins/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/with-markdown-shiki/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/with-mdx/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/with-nanostores/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/with-tailwindcss/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/with-vite-plugin-pwa/.npmrc

This file was deleted.

2 changes: 0 additions & 2 deletions examples/with-vitest/.npmrc

This file was deleted.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"node": "^14.18.0 || >=16.12.0",
"pnpm": ">=7.9.5"
},
"packageManager": "pnpm@7.9.5",
"packageManager": "pnpm@7.12.2",
"pnpm": {
"packageExtensions": {
"svelte2tsx": {
Expand Down Expand Up @@ -74,6 +74,7 @@
"@changesets/changelog-github": "0.4.4",
"@changesets/cli": "2.23.0",
"@octokit/action": "^3.18.1",
"@types/node": "^18.7.21",
"@typescript-eslint/eslint-plugin": "^5.27.1",
"@typescript-eslint/parser": "^5.27.1",
"del": "^7.0.0",
Expand Down
10 changes: 7 additions & 3 deletions packages/astro-rss/src/util.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import npath from 'path-browserify';

/** Normalize URL to its canonical form */
export function createCanonicalURL(url: string, base?: string): URL {
let pathname = url.replace(/\/index.html$/, ''); // index.html is not canonical
pathname = pathname.replace(/\/1\/?$/, ''); // neither is a trailing /1/ (impl. detail of collections)
if (!npath.extname(pathname)) pathname = pathname.replace(/(\/+)?$/, '/'); // add trailing slash if there’s no extension
if (!getUrlExtension(url)) pathname = pathname.replace(/(\/+)?$/, '/'); // add trailing slash if there’s no extension
Copy link
Member Author

@bluwy bluwy Sep 26, 2022

Choose a reason for hiding this comment

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

Question: could we use path.extname from node here? I'm not sure since it imported path-browserify before (maybe just a auto-import issue).

Context: path-browserify was imported by astro-rss before but was not declared as dependency. Thought we could simplify remove the package with a simpler replacement.

pathname = pathname.replace(/\/+/g, '/'); // remove duplicate slashes (URL() won’t)
return new URL(pathname, base);
}
Expand All @@ -17,3 +15,9 @@ export function isValidURL(url: string): boolean {
} catch (e) {}
return false;
}

function getUrlExtension(url: string) {
const lastDot = url.lastIndexOf('.');
const lastSlash = url.lastIndexOf('/');
return lastDot > lastSlash ? url.slice(lastDot + 1) : '';
}
5 changes: 4 additions & 1 deletion packages/astro/e2e/fixtures/astro-component/astro.config.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { defineConfig } from 'astro/config';
import preact from '@astrojs/preact'

// https://astro.build/config
export default defineConfig({});
export default defineConfig({
integrations: [preact()]
});
4 changes: 3 additions & 1 deletion packages/astro/e2e/fixtures/astro-component/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
"@astrojs/preact": "^1.1.0",
"astro": "workspace:*",
"preact": "^10.11.0"
}
}
3 changes: 2 additions & 1 deletion packages/astro/e2e/fixtures/error-cyclic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"version": "0.0.0",
"private": true,
"dependencies": {
"@astrojs/preact": "workspace:*",
"astro": "workspace:*",
"@astrojs/preact": "workspace:*"
"preact": "^10.11.0"
}
}
8 changes: 6 additions & 2 deletions packages/astro/e2e/fixtures/errors/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@
"version": "0.0.0",
"private": true,
"dependencies": {
"@astrojs/react": "workspace:*",
"@astrojs/preact": "workspace:*",
"@astrojs/react": "workspace:*",
"@astrojs/solid-js": "workspace:*",
"@astrojs/svelte": "workspace:*",
"@astrojs/vue": "workspace:*",
"astro": "workspace:*",
"preact": "^10.11.0",
"react": "^18.1.0",
"react-dom": "^18.1.0"
"react-dom": "^18.1.0",
"solid-js": "^1.5.6",
"svelte": "^3.50.1",
"vue": "^3.2.39"
}
}
3 changes: 2 additions & 1 deletion packages/astro/e2e/fixtures/hydration-race/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"build": "astro build"
},
"dependencies": {
"@astrojs/preact": "workspace:*",
"astro": "workspace:*",
"@astrojs/preact": "workspace:*"
"preact": "^10.11.0"
}
}
6 changes: 2 additions & 4 deletions packages/astro/e2e/fixtures/solid-component/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@
"version": "0.0.0",
"private": true,
"dependencies": {
"@astrojs/mdx": "workspace:*",
"@astrojs/solid-js": "workspace:*",
"astro": "workspace:*",
"@astrojs/mdx": "workspace:*"
},
"devDependencies": {
"solid-js": "^1.4.3"
"solid-js": "^1.5.5"
}
}
5 changes: 4 additions & 1 deletion packages/astro/e2e/fixtures/tailwindcss/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
"version": "0.0.0",
"private": true,
"dependencies": {
"@astrojs/tailwind": "workspace:*",
"astro": "workspace:*",
"@astrojs/tailwind": "workspace:*"
"autoprefixer": "^10.4.7",
"postcss": "^8.4.14",
"tailwindcss": "^3.0.24"
}
}
3 changes: 2 additions & 1 deletion packages/astro/e2e/fixtures/vue-component/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
"version": "0.0.0",
"private": true,
"dependencies": {
"@astrojs/mdx": "workspace:*",
"@astrojs/vue": "workspace:*",
"astro": "workspace:*",
"@astrojs/mdx": "workspace:*"
"vue": "^3.2.39"
}
}
10 changes: 9 additions & 1 deletion packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,13 @@
"@types/debug": "^4.1.7",
"@types/diff": "^5.0.2",
"@types/estree": "^0.0.51",
"@types/hast": "^2.3.4",
"@types/mime": "^2.0.3",
"@types/mocha": "^9.1.1",
"@types/parse5": "^6.0.3",
"@types/path-browserify": "^1.0.0",
"@types/prettier": "^2.6.3",
"@types/prompts": "^2.0.14",
"@types/resolve": "^1.20.2",
"@types/rimraf": "^3.0.2",
"@types/send": "^0.17.1",
Expand All @@ -178,8 +180,14 @@
"chai": "^4.3.6",
"cheerio": "^1.0.0-rc.11",
"mocha": "^9.2.2",
"node-fetch": "^3.2.5",
"rehype-autolink-headings": "^6.1.1",
"rehype-slug": "^5.0.1",
"rehype-toc": "^3.0.2",
"remark-code-titles": "^0.1.2",
"sass": "^1.52.2",
"srcset-parse": "^1.1.0"
"srcset-parse": "^1.1.0",
"unified": "^10.1.2"
},
"engines": {
"node": "^14.18.0 || >=16.12.0",
Expand Down
5 changes: 4 additions & 1 deletion packages/astro/playwright.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { devices } from '@playwright/test';
// NOTE: Sometimes, tests fail with `TypeError: process.stdout.clearLine is not a function`
// for some reason. This comes from Vite, and is conditionally called based on `isTTY`.
// We set it to false here to skip this odd behavior.
process.stdout.isTTY = false;
Copy link
Member Author

Choose a reason for hiding this comment

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

I hit this special case locally where e2e is failing on process.stdout.clearLine for some reason. It's not related to the shamefully-hoist change from what I can tell.


const config = {
testMatch: 'e2e/*.test.js',
Expand Down
6 changes: 6 additions & 0 deletions packages/astro/src/core/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,12 @@ async function tryLoadConfig(
optimizeDeps: { entries: [] },
clearScreen: false,
appType: 'custom',
// NOTE: Vite doesn't externalize linked packages by default. During testing locally,
// these dependencies trip up Vite's dev SSR transform. In the future, we should
// avoid `vite.createServer` and use `loadConfigFromFile` instead.
ssr: {
external: ['@astrojs/mdx', '@astrojs/react'],
},
Comment on lines +269 to +271
Copy link
Member Author

@bluwy bluwy Sep 26, 2022

Choose a reason for hiding this comment

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

Added these packages to tryLoadConfig. There's no way to externalize everything always in Vite, so this was the best I could find fixing this.

NOTE: there's a comment explaining this above the highlight change

});
try {
const mod = await viteServer.ssrLoadModule(configPath);
Expand Down
4 changes: 4 additions & 0 deletions packages/astro/src/core/create-vite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ export async function createVite(
},
ssr: {
noExternal: [...getSsrNoExternalDeps(settings.config.root), ...thirdPartyAstroPackages],
// shiki is imported by Code.astro, which is no-externalized (processed by Vite).
// However, shiki's deps are in CJS and trips up Vite's dev SSR transform, externalize
// shiki to load it with node instead.
external: mode === 'dev' ? ['shiki'] : [],
bluwy marked this conversation as resolved.
Show resolved Hide resolved
},
};

Expand Down
3 changes: 0 additions & 3 deletions packages/astro/test/api-routes.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { expect } from 'chai';
import * as cheerio from 'cheerio';
import { loadFixture } from './test-utils.js';
import * as fs from 'fs';
import { FormData, File } from 'node-fetch';

describe('API routes', () => {
/** @type {import('./test-utils').Fixture} */
Expand Down
4 changes: 3 additions & 1 deletion packages/astro/test/fixtures/0-css/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
"@astrojs/vue": "workspace:*",
"astro": "workspace:*",
"react": "^18.1.0",
"react-dom": "^18.1.0"
"react-dom": "^18.1.0",
"svelte": "^3.48.0",
"vue": "^3.2.39"
}
}
3 changes: 2 additions & 1 deletion packages/astro/test/fixtures/alias-tsconfig/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"private": true,
"dependencies": {
"@astrojs/svelte": "workspace:*",
"astro": "workspace:*"
"astro": "workspace:*",
"svelte": "^3.48.0"
}
}
3 changes: 2 additions & 1 deletion packages/astro/test/fixtures/alias/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"private": true,
"dependencies": {
"@astrojs/svelte": "workspace:*",
"astro": "workspace:*"
"astro": "workspace:*",
"svelte": "^3.48.0"
}
}
3 changes: 2 additions & 1 deletion packages/astro/test/fixtures/astro-basic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"private": true,
"dependencies": {
"@astrojs/preact": "workspace:*",
"astro": "workspace:*"
"astro": "workspace:*",
"preact": "^10.11.0"
}
}
5 changes: 4 additions & 1 deletion packages/astro/test/fixtures/astro-children/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
"@astrojs/preact": "workspace:*",
"@astrojs/svelte": "workspace:*",
"@astrojs/vue": "workspace:*",
"astro": "workspace:*"
"astro": "workspace:*",
"preact": "^10.11.0",
"svelte": "^3.48.0",
"vue": "^3.2.39"
}
}
Loading