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

Allow custom const x = boolean exports to be read from plugins #8260

Closed
wants to merge 9 commits into from
Closed
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/brown-tips-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': minor
---

Plugins can now access a page's boolean exports on RouteData['customOptions']
Copy link
Member

@ematipico ematipico Aug 29, 2023

Choose a reason for hiding this comment

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

I would call it externalOptions. It's more sound and communicates that these options don't come from Astro. Or pluginOptions.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should differentiate. If we accept this feature I think it should include all boolean options, including prerender.

The reason is, if this field is just for non-Astro defined booleans, that would mean that if Astro adds any new booleans itself, that becomes a breaking change. However if they are all under customOptions then there is no breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@matthewp The only con to that approach is that we get a huge breaking change now that has a large impact on not only the astro codebase, but also breaks any plugins that rely on prerender. With the other approach, we might introduce breaking changes in the future, but any astro update that introduces a new page option will likely already be a major change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option here is that custom options must have some prefix we define. Similar to how HTML has the data- attributes. If they were required to be customFoo, for example, we could ensure that Astro never accidentally breaks an integration defined option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that idea but custom as a prefix feels a bit bulky to me

1 change: 1 addition & 0 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2108,6 +2108,7 @@ export interface RouteData {
segments: RoutePart[][];
type: RouteType;
prerender: boolean;
customOptions: Record<string, boolean>;
redirect?: RedirectConfig;
redirectRoute?: RouteData;
}
Expand Down
7 changes: 5 additions & 2 deletions packages/astro/src/core/build/plugins/plugin-prerender.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import path from 'node:path';
import type { Plugin as VitePlugin } from 'vite';
import { getPrerenderMetadata } from '../../../prerender/metadata.js';
import { getPageOptionsMetadata } from '../../../prerender/metadata.js';
import type { BuildInternals } from '../internal.js';
import type { AstroBuildPlugin } from '../plugin.js';
import type { StaticBuildOptions } from '../types';
Expand All @@ -19,9 +19,12 @@ function vitePluginPrerender(opts: StaticBuildOptions, internals: BuildInternals
}
const pageInfo = internals.pagesByViteID.get(id);
if (pageInfo) {
const pageOptions = getPageOptionsMetadata(meta.getModuleInfo(id));
pageInfo.route.customOptions = pageOptions?.custom ?? {};

// prerendered pages should be split into their own chunk
// Important: this can't be in the `pages/` directory!
if (getPrerenderMetadata(meta.getModuleInfo(id))) {
if (pageOptions?.prerender) {
pageInfo.route.prerender = true;
return 'prerender';
}
Expand Down
3 changes: 3 additions & 0 deletions packages/astro/src/core/routing/manifest/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ export function createRouteManifest(
generate,
pathname: pathname || undefined,
prerender,
customOptions: {},
});
}
});
Expand Down Expand Up @@ -421,6 +422,7 @@ export function createRouteManifest(
generate,
pathname: pathname || void 0,
prerender: prerenderInjected ?? prerender,
customOptions: {},
});
});

Expand Down Expand Up @@ -458,6 +460,7 @@ export function createRouteManifest(
generate,
pathname: pathname || void 0,
prerender: false,
customOptions: {},
redirect: to,
redirectRoute: routes.find((r) => r.route === to),
};
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/routing/manifest/serialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export function deserializeRouteData(rawRouteData: SerializedRouteData): RouteDa
pathname: rawRouteData.pathname || undefined,
segments: rawRouteData.segments,
prerender: rawRouteData.prerender,
customOptions: rawRouteData.customOptions,
redirect: rawRouteData.redirect,
redirectRoute: rawRouteData.redirectRoute
? deserializeRouteData(rawRouteData.redirectRoute)
Expand Down
9 changes: 5 additions & 4 deletions packages/astro/src/prerender/metadata.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { ModuleInfo, ModuleLoader } from '../core/module-loader';
import { viteID } from '../core/util.js';
import type { PageOptions } from '../vite-plugin-astro/types';

type GetPrerenderStatusParams = {
filePath: URL;
Expand All @@ -13,10 +14,10 @@ export function getPrerenderStatus({
const fileID = viteID(filePath);
const moduleInfo = loader.getModuleInfo(fileID);
if (!moduleInfo) return;
const prerenderStatus = getPrerenderMetadata(moduleInfo);
return prerenderStatus;
const pageOptions = getPageOptionsMetadata(moduleInfo);
return pageOptions?.prerender;
}

export function getPrerenderMetadata(moduleInfo: ModuleInfo) {
return moduleInfo?.meta?.astro?.pageOptions?.prerender;
export function getPageOptionsMetadata(moduleInfo: ModuleInfo) {
return moduleInfo?.meta?.astro?.pageOptions as PageOptions | undefined;
}
1 change: 1 addition & 0 deletions packages/astro/src/vite-plugin-astro/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { PropagationHint } from '../@types/astro';

export interface PageOptions {
prerender?: boolean;
custom?: Record<string, boolean>;
}

export interface PluginMetadata {
Expand Down
47 changes: 28 additions & 19 deletions packages/astro/src/vite-plugin-scanner/scan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ import * as eslexer from 'es-module-lexer';
import { AstroError, AstroErrorData } from '../core/errors/index.js';

const BOOLEAN_EXPORTS = new Set(['prerender']);
type BuiltInExports = Exclude<keyof PageOptions, 'custom'>;

// Quick scan to determine if code includes recognized export
// False positives are not a problem, so be forgiving!
function includesExport(code: string) {
for (const name of BOOLEAN_EXPORTS) {
if (code.includes(name)) return true;
}
return false;
return code.includes('export const');
}

// Support quoted values to allow statically known `import.meta.env` variables to be used
Expand Down Expand Up @@ -51,33 +52,41 @@ export async function scan(

let pageOptions: PageOptions = {};
for (const _export of exports) {
const { n: name, le: endOfLocalName } = _export;
// mark that a `prerender` export was found
if (BOOLEAN_EXPORTS.has(name)) {
// For a given export, check the value of the local declaration
// Basically extract the `const` from the statement `export const prerender = true`
const prefix = code
.slice(0, endOfLocalName)
.split('export')
.pop()!
.trim()
.replace('prerender', '')
.trim();
// For a given export, check the value of the first non-whitespace token.
// Basically extract the `true` from the statement `export const prerender = true`
const suffix = code.slice(endOfLocalName).trim().replace(/\=/, '').trim().split(/[;\n]/)[0];
if (prefix !== 'const' || !(isTruthy(suffix) || isFalsy(suffix))) {
const { n: name, ls: startOfLocalName, le: endOfLocalName } = _export;
if (startOfLocalName === -1 || endOfLocalName === -1) continue;
ottomated marked this conversation as resolved.
Show resolved Hide resolved
// For a given export, check the value of the local declaration
// Basically extract the `const` from the statement `export const prerender = true`
const indexOfExport = code.lastIndexOf('export', startOfLocalName);
const prefix = code.slice(indexOfExport + /* "export".length */ 6, startOfLocalName).trim();

// `export const name = "value";
// ^ ^
// indexOfFirstNonWhitespace endOfLineIndex
const indexOfFirstNonWhitespace = code.slice(endOfLocalName).search(/[^=\s]/) + endOfLocalName;
const endOfLineIndex = code.slice(indexOfFirstNonWhitespace).search(/[\n;]/) + indexOfFirstNonWhitespace;
ottomated marked this conversation as resolved.
Show resolved Hide resolved
const exportValue = code.slice(indexOfFirstNonWhitespace, endOfLineIndex).trim();

const isBuiltIn = BOOLEAN_EXPORTS.has(name);

if (prefix !== 'const' || !(isTruthy(exportValue) || isFalsy(exportValue))) {
// Only throw an error for built-in exports
if (isBuiltIn) {
throw new AstroError({
...AstroErrorData.InvalidPrerenderExport,
message: AstroErrorData.InvalidPrerenderExport.message(
prefix,
suffix,
exportValue,
settings?.config.output === 'hybrid' ?? false
),
location: { file: id },
});
}
} else {
if (isBuiltIn) {
pageOptions[name as BuiltInExports] = isTruthy(exportValue);
} else {
pageOptions[name as keyof PageOptions] = isTruthy(suffix);
pageOptions.custom = pageOptions.custom ?? {};
pageOptions.custom[name] = isTruthy(exportValue);
}
}
}
Expand Down
40 changes: 40 additions & 0 deletions packages/astro/test/custom-page-options.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { expect } from 'chai';
import { loadFixture } from './test-utils.js';

describe('Public', () => {
let fixture;
let routes;

const integration = {
name: 'test-custom-page-options',
hooks: {
['astro:build:done']: (options) => {
routes = options.routes;
},
},
}

before(async () => {
fixture = await loadFixture({
root: './fixtures/custom-page-options/',
integrations: [integration],
});
await fixture.build();
});

it('sets custom options', async () => {
const page = routes.find(r => r.pathname === '/');
expect(page.customOptions).to.deep.equal({
customTrue: true,
customFalse: false,
whitespace: true,
// customString: 'string',
// customString2: 'string2',
// customNumber: 123,
// customNumber2: -123.456,
// customHexNumber: 0xa,
// customOctalNumber: 0o10,
// customBinaryNumber: 0b1010,
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { defineConfig } from 'astro/config';

// https://astro.build/config
export default defineConfig({});
8 changes: 8 additions & 0 deletions packages/astro/test/fixtures/custom-page-options/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@test/custom-page-options",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
export const prerender = true;

export const customTrue = true;
export const customFalse = false;
ottomated marked this conversation as resolved.
Show resolved Hide resolved
export const whitespace = true
export let invalid = true;

// For later?
export const customString = 'string';
export const customString2 = "string2";
export const customNumber = 123;
export const customNumber2 = -123.456;
export const customHexNumber = 0xa;
export const customOctalNumber = 0o10;
export const customBinaryNumber = 0b1010;
---
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.