Skip to content

Commit

Permalink
Merge branch 'withastro:main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
Kynson authored Jan 18, 2025
2 parents a401ad7 + 22eafff commit ff96fec
Show file tree
Hide file tree
Showing 16 changed files with 113 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/many-pianos-develop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/react': patch
---

Removes hardcoded `ssr.external: ['react-dom/server', 'react-dom/client']` config that causes issues with adapters that bundle all dependencies (e.g. Cloudflare). These externals should already be inferred by default by Vite when deploying to a server environment.
5 changes: 5 additions & 0 deletions .changeset/orange-suits-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes an issue where server islands do not work in projects that use an adapter but only have prerendered pages. If an adapter is added, the server island endpoint will now be added by default.
5 changes: 5 additions & 0 deletions .changeset/warm-pandas-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes a bug that caused Astro to attempt to inject environment variables into non-source files, causing performance problems and broken builds
13 changes: 7 additions & 6 deletions packages/astro/src/core/routing/manifest/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -718,12 +718,13 @@ export async function createRouteManifest(
}
if (dev || settings.buildOutput === 'server') {
injectImageEndpoint(settings, { routes }, dev ? 'dev' : 'build');
// Ideally we would only inject the server islands route if server islands are used in the project.
// Unfortunately, there is a "circular dependency": to know if server islands are used, we need to run
// the build but the build relies on the routes manifest.
// This situation also means we cannot update the buildOutput based on whether or not server islands
// are used in the project. If server islands are detected after the build but the buildOutput is
// static, we fail the build.
}

// If an adapter is added, we unconditionally inject the server islands route.
// Ideally we would only inject the server islands route if server islands are used in the project.
// Unfortunately, there is a "circular dependency": to know if server islands are used, we need to run
// the build but the build relies on the routes manifest.
if (dev || settings.config.adapter) {
injectServerIslandRoute(settings.config, { routes });
}
await runHookRoutesResolved({ routes, settings, logger });
Expand Down
12 changes: 10 additions & 2 deletions packages/astro/src/env/vite-plugin-import-meta-env.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { transform } from 'esbuild';
import MagicString from 'magic-string';
import type * as vite from 'vite';
import { createFilter, isCSSRequest } from 'vite';
import type { EnvLoader } from './env-loader.js';

interface EnvPluginOptions {
Expand Down Expand Up @@ -71,6 +72,7 @@ export function importMetaEnv({ envLoader }: EnvPluginOptions): vite.Plugin {
let isDev: boolean;
let devImportMetaEnvPrepend: string;
let viteConfig: vite.ResolvedConfig;
const filter = createFilter(null, ['**/*.html', '**/*.htm', '**/*.json']);
return {
name: 'astro:vite-plugin-env',
config(_, { command }) {
Expand All @@ -96,11 +98,17 @@ export function importMetaEnv({ envLoader }: EnvPluginOptions): vite.Plugin {
}
}
},

transform(source, id, options) {
if (!options?.ssr || !source.includes('import.meta.env')) {
if (
!options?.ssr ||
!source.includes('import.meta.env') ||
!filter(id) ||
isCSSRequest(id) ||
viteConfig.assetsInclude(id)
) {
return;
}

// Find matches for *private* env and do our own replacement.
// Env is retrieved before process.env is populated by astro:env
// so that import.meta.env is first replaced by values, not process.env
Expand Down
9 changes: 9 additions & 0 deletions packages/astro/test/astro-envs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,5 +120,14 @@ describe('Environment Variables', () => {
let $ = cheerio.load(indexHtml);
assert.equal($('#base-url').text(), '/blog');
});

it('does not inject env into imported asset files', async () => {
let res = await fixture.fetch('/blog/');
assert.equal(res.status, 200);
let indexHtml = await res.text();
let $ = cheerio.load(indexHtml);
assert.equal($('#env').text(), 'A MYSTERY');
assert.equal($('#css').text(), 'good');
});
});
});
1 change: 1 addition & 0 deletions packages/astro/test/fixtures/astro-envs/.env
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
SECRET_PLACE=CLUB_33
PUBLIC_PLACE=BLUE_BAYOU
KITTY=CHESHIRE
14 changes: 14 additions & 0 deletions packages/astro/test/fixtures/astro-envs/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,18 @@ export default defineConfig({
site: 'http://example.com',
base: '/blog',
integrations: [vue()],
vite: {
plugins: [
{
// Plugin so that we can see in the tests whether the env has been injected
name: 'export-env-plugin',
enforce: 'post',
transform(code, id) {
if (id.endsWith('.json')) {
return `${code}\n export const env = ${JSON.stringify(code.includes('CHESHIRE') || code.includes('process.env.KITTY') ? 'CHESHIRE' : 'A MYSTERY')}`;
}
},
},
],
},
});
27 changes: 27 additions & 0 deletions packages/astro/test/fixtures/astro-envs/src/data/cats.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"tiddles": {
"name": "Tiddles",
"age": 3,
"colour": "black"
},
"mittens": {
"name": "Mittens",
"age": 5,
"colour": "white"
},
"fluffy": {
"name": "Fluffy",
"age": 2,
"colour": "grey"
},
"whiskers": {
"name": "Whiskers",
"age": 4,
"colour": "tabby"
},
"bobby-env": {
"name": "import.meta.env.KITTY",
"age": 1,
"colour": "calico"
}
}
4 changes: 4 additions & 0 deletions packages/astro/test/fixtures/astro-envs/src/data/hello.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/* Just mentioning import.meta.env is enough to trigger this */
body {
background-color: red;
}
7 changes: 7 additions & 0 deletions packages/astro/test/fixtures/astro-envs/src/data/hi.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
---
<h1>import.meta.env.KITTEN</h1>

```js
console.log(import.meta.env.KITTEN)
```
5 changes: 5 additions & 0 deletions packages/astro/test/fixtures/astro-envs/src/pages/index.astro
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
---
import Client from '../components/Client.vue';
import css from '../data/hello.css?inline';
const {env} = await import('../data/cats.json');
---
<head />
<environment-variable>{import.meta.env.PUBLIC_PLACE}</environment-variable>
<environment-variable>{import.meta.env.SECRET_PLACE}</environment-variable>
<environment-variable>{import.meta.env.SITE}</environment-variable>
<environment-variable id="base-url">{import.meta.env.BASE_URL}</environment-variable>
<environment-variable id="env">{env}</environment-variable>
<environment-variable id="css">{css.includes('SECRET_PLACE') ? 'bad' : 'good' }</environment-variable>
<Client client:load />
14 changes: 14 additions & 0 deletions packages/astro/test/fixtures/astro-envs/src/pages/info.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Document</title>
</head>

<body>
Did you know import.meta.env is a magic word?
</body>

</html>
9 changes: 0 additions & 9 deletions packages/astro/test/units/routing/manifest.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,6 @@ describe('routing - createRouteManifest', () => {
});

assert.deepEqual(getManifestRoutes(manifest), [
{
route: '/_server-islands/[name]',
type: 'page',
},
{
route: '/_image',
type: 'endpoint',
Expand Down Expand Up @@ -314,10 +310,6 @@ describe('routing - createRouteManifest', () => {
});

assert.deepEqual(getManifestRoutes(manifest), [
{
route: '/_server-islands/[name]',
type: 'page',
},
{
route: '/_image',
type: 'endpoint',
Expand Down Expand Up @@ -457,7 +449,6 @@ describe('routing - createRouteManifest', () => {
});

assert.deepEqual(getManifestRoutes(manifest), [
{ type: 'page', route: '/_server-islands/[name]' },
{ type: 'endpoint', route: '/_image' },
{ type: 'endpoint', route: '/blog/a-[b].233' },
{ type: 'redirect', route: '/posts/a-[b].233' },
Expand Down
1 change: 0 additions & 1 deletion packages/integrations/react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ function getViteConfiguration(
},
plugins: [react({ include, exclude, babel }), optionsPlugin(!!experimentalReactChildren)],
ssr: {
external: reactConfig.externals,
noExternal: [
// These are all needed to get mui to work.
'@mui/material',
Expand Down
3 changes: 0 additions & 3 deletions packages/integrations/react/src/version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,13 @@ export const versionsConfig = {
17: {
server: '@astrojs/react/server-v17.js',
client: '@astrojs/react/client-v17.js',
externals: ['react-dom/server.js', 'react-dom/client.js'],
},
18: {
server: '@astrojs/react/server.js',
client: '@astrojs/react/client.js',
externals: ['react-dom/server', 'react-dom/client'],
},
19: {
server: '@astrojs/react/server.js',
client: '@astrojs/react/client.js',
externals: ['react-dom/server', 'react-dom/client'],
},
};

0 comments on commit ff96fec

Please sign in to comment.