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

Use esbuild for env replacement #9652

Merged
merged 11 commits into from
Jan 11, 2024
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/lazy-pandas-pretend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@astrojs/mdx": patch
---

Removes environment variables workaround that broke project builds with sourcemaps
5 changes: 5 additions & 0 deletions .changeset/silent-pandas-rush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Improves environment variables handling by using esbuild to perform replacements
9 changes: 4 additions & 5 deletions packages/astro/src/content/vite-plugin-content-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { getProxyCode } from '../assets/utils/proxy.js';
import { AstroError } from '../core/errors/errors.js';
import { AstroErrorData } from '../core/errors/index.js';
import { isServerLikeOutput } from '../prerender/utils.js';
import { escapeViteEnvReferences } from '../vite-plugin-utils/index.js';
import { CONTENT_FLAG, DATA_FLAG } from './consts.js';
import {
getContentEntryExts,
Expand Down Expand Up @@ -93,7 +92,7 @@ export function astroContentImportPlugin({
pluginContext: this,
});

const code = escapeViteEnvReferences(`
const code = `
export const id = ${JSON.stringify(id)};
export const collection = ${JSON.stringify(collection)};
export const data = ${stringifyEntryData(data, isServerLikeOutput(settings.config))};
Expand All @@ -102,7 +101,7 @@ export const _internal = {
filePath: ${JSON.stringify(_internal.filePath)},
rawData: ${JSON.stringify(_internal.rawData)},
};
`);
`;
return code;
} else if (hasContentFlag(viteId, CONTENT_FLAG)) {
const fileId = viteId.split('?')[0];
Expand All @@ -115,7 +114,7 @@ export const _internal = {
pluginContext: this,
});

const code = escapeViteEnvReferences(`
const code = `
export const id = ${JSON.stringify(id)};
export const collection = ${JSON.stringify(collection)};
export const slug = ${JSON.stringify(slug)};
Expand All @@ -125,7 +124,7 @@ export const _internal = {
type: 'content',
filePath: ${JSON.stringify(_internal.filePath)},
rawData: ${JSON.stringify(_internal.rawData)},
};`);
};`;

return { code, map: { mappings: '' } };
}
Expand Down
166 changes: 119 additions & 47 deletions packages/astro/src/vite-plugin-env/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
import MagicString from 'magic-string';
import { fileURLToPath } from 'node:url';
import type * as vite from 'vite';
import { loadEnv } from 'vite';
import { transform } from 'esbuild';
import MagicString from 'magic-string';
import type { AstroConfig, AstroSettings } from '../@types/astro.js';

interface EnvPluginOptions {
settings: AstroSettings;
}

// Match `import.meta.env` directly without trailing property access
const importMetaEnvOnlyRe = /\bimport\.meta\.env\b(?!\.)/;
// Match valid JS variable names (identifiers), which accepts most alphanumeric characters,
// except that the first character cannot be a number.
const isValidIdentifierRe = /^[_$a-zA-Z][_$a-zA-Z0-9]*$/;
bluwy marked this conversation as resolved.
Show resolved Hide resolved

function getPrivateEnv(
viteConfig: vite.ResolvedConfig,
astroConfig: AstroConfig
Expand All @@ -29,7 +36,7 @@ function getPrivateEnv(
const privateEnv: Record<string, string> = {};
for (const key in fullEnv) {
// Ignore public env var
if (envPrefixes.every((prefix) => !key.startsWith(prefix))) {
if (isValidIdentifierRe.test(key) && envPrefixes.every((prefix) => !key.startsWith(prefix))) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Added a valid identifier check because keys could be ProgramFiles(x64) which wouldn't work as import.meta.env.ProgramFiles(x64). This worked before because we do plain string replacements, but I don't think people were relying on this to work.

if (typeof process.env[key] !== 'undefined') {
let value = process.env[key];
// Replacements are always strings, so try to convert to strings here first
Expand Down Expand Up @@ -61,71 +68,136 @@ function getReferencedPrivateKeys(source: string, privateEnv: Record<string, any
return references;
}

export default function envVitePlugin({ settings }: EnvPluginOptions): vite.PluginOption {
/**
* Use esbuild to perform replacememts like Vite
* https://github.com/vitejs/vite/blob/5ea9edbc9ceb991e85f893fe62d68ed028677451/packages/vite/src/node/plugins/define.ts#L130
*/
async function replaceDefine(
code: string,
id: string,
define: Record<string, string>,
config: vite.ResolvedConfig
): Promise<{ code: string; map: string | null }> {
// Since esbuild doesn't support replacing complex expressions, we replace `import.meta.env`
// with a marker string first, then postprocess and apply the `Object.assign` code.
const replacementMarkers: Record<string, string> = {};
const env = define['import.meta.env'];
if (env) {
// Compute the marker from the length of the replaced code. We do this so that esbuild generates
// the sourcemap with the right column offset when we do the postprocessing.
const marker = `__astro_import_meta_env${'_'.repeat(
env.length - 23 /* length of preceding string */
)}`;
replacementMarkers[marker] = env;
define = { ...define, 'import.meta.env': marker };
}

const esbuildOptions = config.esbuild || {};

const result = await transform(code, {
loader: 'js',
charset: esbuildOptions.charset ?? 'utf8',
platform: 'neutral',
define,
sourcefile: id,
sourcemap: config.command === 'build' ? !!config.build.sourcemap : true,
});

for (const marker in replacementMarkers) {
result.code = result.code.replaceAll(marker, replacementMarkers[marker]);
}

return {
code: result.code,
map: result.map || null,
};
}

export default function envVitePlugin({ settings }: EnvPluginOptions): vite.Plugin {
let privateEnv: Record<string, string>;
let defaultDefines: Record<string, string>;
let isDev: boolean;
let devImportMetaEnvPrepend: string;
let viteConfig: vite.ResolvedConfig;
const { config: astroConfig } = settings;
return {
name: 'astro:vite-plugin-env',
enforce: 'pre',
config(_, { command }) {
isDev = command !== 'build';
},
configResolved(resolvedConfig) {
viteConfig = resolvedConfig;

// HACK: move ourselves before Vite's define plugin to apply replacements at the right time (before Vite normal plugins)
Copy link
Member Author

Choose a reason for hiding this comment

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

Copied this hack from @astrojs/mdx.

const viteDefinePluginIndex = resolvedConfig.plugins.findIndex(
(p) => p.name === 'vite:define'
);
if (viteDefinePluginIndex !== -1) {
const myPluginIndex = resolvedConfig.plugins.findIndex(
(p) => p.name === 'astro:vite-plugin-env'
);
if (myPluginIndex !== -1) {
const myPlugin = resolvedConfig.plugins[myPluginIndex];
// @ts-ignore-error ignore readonly annotation
resolvedConfig.plugins.splice(viteDefinePluginIndex, 0, myPlugin);
// @ts-ignore-error ignore readonly annotation
resolvedConfig.plugins.splice(myPluginIndex, 1);
}
}
},
async transform(source, id, options) {
transform(source, id, options) {
if (!options?.ssr || !source.includes('import.meta.env')) {
return;
}

// Find matches for *private* env and do our own replacement.
let s: MagicString | undefined;
const pattern = new RegExp(
// Do not allow preceding '.', but do allow preceding '...' for spread operations
'(?<!(?<!\\.\\.)\\.)\\b(' +
// Captures `import.meta.env.*` calls and replace with `privateEnv`
`import\\.meta\\.env\\.(.+?)` +
'|' +
// This catches destructed `import.meta.env` calls,
// BUT we only want to inject private keys referenced in the file.
// We overwrite this value on a per-file basis.
'import\\.meta\\.env' +
// prevent trailing assignments
')\\b(?!\\s*?=[^=])',
'g'
);
let references: Set<string>;
let match: RegExpExecArray | null;

while ((match = pattern.exec(source))) {
let replacement: string | undefined;
// If we match exactly `import.meta.env`, define _only_ referenced private variables
if (match[0] === 'import.meta.env') {
privateEnv ??= getPrivateEnv(viteConfig, astroConfig);
references ??= getReferencedPrivateKeys(source, privateEnv);
replacement = `(Object.assign(import.meta.env,{`;
for (const key of references.values()) {
replacement += `${key}:${privateEnv[key]},`;
privateEnv ??= getPrivateEnv(viteConfig, astroConfig);

// In dev, we can assign the private env vars to `import.meta.env` directly for performance
if (isDev) {
const s = new MagicString(source);
if (!devImportMetaEnvPrepend) {
devImportMetaEnvPrepend = `Object.assign(import.meta.env,{`;
for (const key in privateEnv) {
devImportMetaEnvPrepend += `${key}:${privateEnv[key]},`;
}
replacement += '}))';
}
// If we match `import.meta.env.*`, replace with private env
else if (match[2]) {
privateEnv ??= getPrivateEnv(viteConfig, astroConfig);
replacement = privateEnv[match[2]];
devImportMetaEnvPrepend += '});';
}
if (replacement) {
const start = match.index;
const end = start + match[0].length;
s ??= new MagicString(source);
s.overwrite(start, end, replacement);
}
}

if (s) {
s.prepend(devImportMetaEnvPrepend);
return {
code: s.toString(),
map: s.generateMap({ hires: 'boundary' }),
};
}

// In build, use esbuild to perform replacements. Compute the default defines for esbuild here as a
// separate object as it could be extended by `import.meta.env` later.
if (!defaultDefines) {
defaultDefines = {};
for (const key in privateEnv) {
defaultDefines[`import.meta.env.${key}`] = privateEnv[key];
}
}

let defines = defaultDefines;

// If reference the `import.meta.env` object directly, we want to inject private env vars
// into Vite's injected `import.meta.env` object. To do this, we use `Object.assign` and keeping
// the `import.meta.env` identifier so Vite sees it.
if (importMetaEnvOnlyRe.test(source)) {
const references = getReferencedPrivateKeys(source, privateEnv);
let replacement = `(Object.assign(import.meta.env,{`;
for (const key of references.values()) {
replacement += `${key}:${privateEnv[key]},`;
}
replacement += '}))';
defines = {
...defaultDefines,
'import.meta.env': replacement,
};
}

return replaceDefine(source, id, defines, viteConfig);
},
};
}
6 changes: 3 additions & 3 deletions packages/astro/src/vite-plugin-markdown/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import type { Logger } from '../core/logger/core.js';
import { isMarkdownFile } from '../core/util.js';
import { shorthash } from '../runtime/server/shorthash.js';
import type { PluginMetadata } from '../vite-plugin-astro/types.js';
import { escapeViteEnvReferences, getFileInfo } from '../vite-plugin-utils/index.js';
import { getFileInfo } from '../vite-plugin-utils/index.js';
import { getMarkdownCodeForImages, type MarkdownImagePath } from './images.js';

interface AstroPluginOptions {
Expand Down Expand Up @@ -116,7 +116,7 @@ export default function markdown({ settings, logger }: AstroPluginOptions): Plug
);
}

const code = escapeViteEnvReferences(`
const code = `
import { unescapeHTML, spreadAttributes, createComponent, render, renderComponent, maybeRenderHead } from ${JSON.stringify(
astroServerRuntimeModulePath
)};
Expand Down Expand Up @@ -166,7 +166,7 @@ export default function markdown({ settings, logger }: AstroPluginOptions): Plug
}
});
export default Content;
`);
`;

return {
code,
Expand Down
11 changes: 0 additions & 11 deletions packages/astro/src/vite-plugin-utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,6 @@ import {
} from '../core/path.js';
import { viteID } from '../core/util.js';

/**
* Converts the first dot in `import.meta.env` to its Unicode escape sequence,
* which prevents Vite from replacing strings like `import.meta.env.SITE`
* in our JS representation of modules like Markdown
*/
export function escapeViteEnvReferences(code: string) {
return code
.replace(/import\.meta\.env/g, 'import\\u002Emeta.env')
.replace(/process\.env/g, 'process\\u002Eenv');
}

export function getFileInfo(id: string, config: AstroConfig) {
const sitePathname = appendForwardSlash(
config.site ? new URL(config.base, config.site).pathname : config.base
Expand Down
11 changes: 2 additions & 9 deletions packages/integrations/mdx/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export default function mdx(partialMdxOptions: Partial<MdxOptions> = {}): AstroI
const compiled = await processor.process(vfile);

return {
code: escapeViteEnvReferences(String(compiled.value)),
code: String(compiled.value),
Copy link
Member Author

Choose a reason for hiding this comment

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

We can safely remove this in a non breaking way as the plugin calls fs.readFile, bypassing Vite's transform. The only continuing env replacement left after this transform is Vite's define, which we know is fixed in Vite 5.

Luckily Astro's env replacements (which didn't work well in plain strings) happen before the fs.readFile, so the broken code was thrown away.

map: compiled.map,
};
} catch (e: any) {
Expand Down Expand Up @@ -215,7 +215,7 @@ export default function mdx(partialMdxOptions: Partial<MdxOptions> = {}): AstroI
import.meta.hot.decline();
}`;
}
return { code: escapeViteEnvReferences(code), map: null };
return { code, map: null };
},
},
] as VitePlugin[],
Expand Down Expand Up @@ -262,10 +262,3 @@ function applyDefaultOptions({
optimize: options.optimize ?? defaults.optimize,
};
}

// Converts the first dot in `import.meta.env` to its Unicode escape sequence,
// which prevents Vite from replacing strings like `import.meta.env.SITE`
// in our JS representation of loaded Markdown files
function escapeViteEnvReferences(code: string) {
return code.replace(/import\.meta\.env/g, 'import\\u002Emeta.env');
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export function recmaInjectImportMetaEnv({
if (node.type === 'MemberExpression') {
// attempt to get "import.meta.env" variable name
const envVarName = getImportMetaEnvVariableName(node);
if (typeof envVarName === 'string') {
if (typeof envVarName === 'string' && importMetaEnv[envVarName] != null) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This check was the main reason for the error in #9012, since if it's undefined, we accidentally generate a broken AST.

After fixing this, it relies on other fixes in this PR so that it works correctly in runtime.

// clear object keys to replace with envVarLiteral
for (const key in node) {
delete (node as any)[key];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,11 @@ export default {
syntaxHighlight: false,
},
integrations: [mdx()],
vite: {
build: {
// Enabling sourcemap may crash the build when using `import.meta.env.UNKNOWN_VAR`
// https://github.com/withastro/astro/issues/9012
sourcemap: true,
},
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ title: Let's talk about my import.meta.env.SITE
export const modeWorks =
import.meta.env.MODE === 'production' ? 'MODE works' : 'MODE does not work!';

export const unknownVar = import.meta.env.UNKNOWN_VAR;

# About my import.meta.env.SITE

My `import.meta.env.SITE` is so cool, I can put env variables in code!
Expand All @@ -27,6 +29,12 @@ I can also use `import.meta.env` in variable exports: {modeWorks}

</div>

<div data-env-variable-exports-unknown>

I can also use `import.meta.env.UNKNOWN_VAR` through exports: "{unknownVar}"

</div>

I can also use vars as HTML attributes:

<div
Expand Down
3 changes: 3 additions & 0 deletions packages/integrations/mdx/test/mdx-vite-env-vars.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ describe('MDX - Vite env vars', () => {
expect(document.querySelector('[data-env-variable-exports]')?.innerHTML).to.contain(
'MODE works'
);
expect(document.querySelector('[data-env-variable-exports-unknown]')?.innerHTML).to.contain(
'exports: ””' // NOTE: these double quotes are special unicode quotes emitted in the HTML file
);
});
it('Transforms `import.meta.env` in HTML attributes', async () => {
const html = await fixture.readFile('/vite-env-vars/index.html');
Expand Down