Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 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
2 changes: 1 addition & 1 deletion deno.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "veryfront",
"version": "0.1.1203",
"version": "0.1.1204",
"license": "Apache-2.0",
"nodeModulesDir": "auto",
"minimumDependencyAge": {
Expand Down
7 changes: 5 additions & 2 deletions extensions/ext-parser-babel/src/parser-only.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export interface BabelParseOnlyParserContract {

function pickPlugins(filePath?: string): parser.ParserPlugin[] {
const normalizedPath = filePath?.toLowerCase() ?? "";
const isTypeScript = /\.(?:tsx?|[cm]ts)$/.test(normalizedPath);
const supportsJsx = !filePath ||
/\.(?:tsx|jsx|js|mjs|cjs)$/.test(normalizedPath);
const plugins: parser.ParserPlugin[] = [
Expand All @@ -33,8 +32,12 @@ function pickPlugins(filePath?: string): parser.ParserPlugin[] {
"dynamicImport",
"importAttributes",
"topLevelAwait",
// Hosted configs are authored in TypeScript but can arrive named `.js`, so
// the extension cannot decide the dialect. TypeScript is a superset, so
// enabling it always only widens what parses.
"typescript",
];
if (isTypeScript || !filePath) plugins.push("typescript");
// JSX stays extension-driven so `.ts` keeps `<T>x` as a type assertion.
if (supportsJsx) plugins.push("jsx");
return plugins;
}
Expand Down
47 changes: 47 additions & 0 deletions src/config/declarative-evaluator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,53 @@ export default defineConfig({
assertEquals(reachableNpmNames.has("@redis/client"), false);
});

it("accepts TypeScript syntax in a config named .js or .mjs", async () => {
// Regression: hosted configs are authored in TypeScript but can be served
// under a .js name. The parser chooses its plugins from the file extension,
// so passing the name parsed `as const` as plain JavaScript and rejected
// valid config with "Hosted configuration rejected (syntax-error:
// syntax-error)", which took customer sites down.
//
// The suite never caught it because DeclarativeConfigFileName defaults to
// veryfront.config.ts, so every other test here implicitly picked the one
// extension that works.
const source = `
import { defineConfig } from "veryfront";

const router = "pages" as const;

export default defineConfig({
title: "TS syntax under a JS name",
router,
});
`;

const asJs = await evaluateDeclarativeConfig({
...DEFAULT_OPTIONS,
fileName: "veryfront.config.js",
source,
});
assertEquals(asJs.router, "pages", "veryfront.config.js must parse TypeScript syntax");

const asMjs = await evaluateDeclarativeConfig({
...DEFAULT_OPTIONS,
fileName: "veryfront.config.mjs",
source,
});
assertEquals(asMjs.router, "pages", "veryfront.config.mjs must parse TypeScript syntax");
});

it("still allows angle-bracket type assertions in a .ts config", async () => {
// Withholding filePath would put every config in TSX mode, where `<T>x` is
// an unclosed JSX element rather than a type assertion.
const snapshot = await evaluateDeclarativeConfig({
...DEFAULT_OPTIONS,
fileName: "veryfront.config.ts",
source: 'const router = <string> "pages";\nexport default { router };',
});
assertEquals(snapshot.router, "pages");
});

it("supports helper aliases, safe spreads, environment branching, templates, and TS wrappers", async () => {
const snapshot = await evaluateDeclarativeConfig({
source: `
Expand Down
8 changes: 8 additions & 0 deletions src/config/declarative-evaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3598,6 +3598,14 @@ async function evaluateCapturedInput(
const { source, fileName, preparedState } = input;
let parsedAst: unknown;
try {
// The name is not reliably the file we are holding: VERYFRONT_CONFIG_FILES
// is ordered `.js, .ts, .mjs`, and in production a project's
// veryfront.config.ts was evaluated under the `.js` name. pickPlugins now
// enables TypeScript regardless of extension, so the name only selects JSX.
//
// The mislabeling itself is a separate defect and is still unfixed:
// readHostedConfigSource returns the candidate it actually loaded, so the
// substitution happens downstream of it on the API-backed path.
parsedAst = await parser.parse({
code: source,
filePath: fileName,
Expand Down

Large diffs are not rendered by default.

72 changes: 72 additions & 0 deletions src/html/styles-builder/css-import-extraction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,78 @@ describe("html/styles-builder/css-import-extraction", () => {
]);
});

it("ignores identifiers that merely contain the word import", () => {
// Without a word boundary, `important` reads as an import statement. In a
// release-asset build a bogus specifier becomes a fatal coverage gap, so
// a false positive here fails the whole release.
assertEquals(extractCssImportSpecifiers('const important = "./styles.css";'), []);
assertEquals(extractCssImportSpecifiers('let unimportant = "./a.css";'), []);
// The real thing still matches, including with no space before the quote.
assertEquals(extractCssImportSpecifiers('import"./styles.css";'), ["./styles.css"]);
});

it("ignores imports that are not code", () => {
// These are not ignored merely as a nicety. A release-asset build records
// any specifier it cannot resolve as a coverage gap, and gaps abort the
// build -- so a commented-out import to a since-deleted stylesheet would
// block that project's releases permanently.
assertEquals(extractCssImportSpecifiers('// import "./legacy.css";'), []);
assertEquals(extractCssImportSpecifiers('/* import "./legacy.css"; */'), []);
assertEquals(
extractCssImportSpecifiers('/*\n * import "./legacy.css";\n */'),
[],
);
assertEquals(extractCssImportSpecifiers('const t = `import "./legacy.css"`;'), []);
assertEquals(
extractCssImportSpecifiers('```tsx\nimport "./theme.css";\n```'),
[],
);
});

it("does not treat import.meta as an import statement", () => {
// `import` followed by a `.css` string later in the same statement used to
// match, because nothing required the keyword to begin a declaration.
assertEquals(
extractCssImportSpecifiers('console.log(import.meta.url, "./styles.css");'),
[],
);
assertEquals(
extractCssImportSpecifiers('const u = import.meta.resolve("./a.css");'),
[],
);
});

it("matches dynamic imports, which are real CSS imports", () => {
// Pinned deliberately. `import("./theme.css")` loads that stylesheet at
// runtime, so dropping it would leave the compiled stylesheet missing CSS
// the page uses. A dynamic specifier naming a file that does not exist is
// a broken reference, not a false positive -- same as a static one.
assertEquals(
extractCssImportSpecifiers('const load = () => import("./theme.css");'),
["./theme.css"],
);
assertEquals(extractCssImportSpecifiers('await import("./a.css");'), ["./a.css"]);
// Still excluded, because that is a property access rather than an import.
assertEquals(extractCssImportSpecifiers('import.meta.resolve("./a.css");'), []);
});

it("still finds real imports alongside non-code lookalikes", () => {
const source = [
'// import "./commented.css";',
'import "./real.css";',
'const sample = `import "./template.css"`;',
'import styles from "./mod.module.css";',
].join("\n");
assertEquals(extractCssImportSpecifiers(source), ["./real.css", "./mod.module.css"]);
});

it("keeps a URL in a string from reading as a comment", () => {
assertEquals(
extractCssImportSpecifiers('const cdn = "https://x.dev";\nimport "./real.css";'),
["./real.css"],
);
});

it("does not match specifiers across statement boundaries", () => {
const source = 'const a = 1; import { b } from "./b.ts"; const s = "x.css";';
assertEquals(extractCssImportSpecifiers(source), []);
Expand Down
45 changes: 41 additions & 4 deletions src/html/styles-builder/css-import-extraction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,54 @@ import { isWithinDirectory, normalizePath } from "#veryfront/utils/path-utils.ts
export const CSS_IMPORTING_SOURCE_EXTENSIONS = [".tsx", ".jsx", ".mdx", ".ts", ".js"];

/**
* Static ESM import statements whose specifier ends in `.css`:
* ESM imports whose specifier ends in `.css`:
* import "./styles.css";
* import styles from "./button.module.css";
* `[^'";]*` keeps the match from crossing statement boundaries.
* import("./theme.css")
*
* Dynamic imports are matched on purpose, despite this once being described as
* static-only. `import("./theme.css")` loads that stylesheet at runtime, so
* leaving it out means the compiled stylesheet is missing CSS the page actually
* uses. A dynamic specifier pointing at a file that does not exist is a broken
* reference, not a false positive -- exactly as a static one would be.
* `[^'";]*` keeps the match from crossing statement boundaries, and `\bimport\b`
* keeps identifiers that merely contain the word out of it -- without it,
* `const important = "./styles.css"` reads as an import. That matters more here
* than it looks: release-asset builds turn a bogus specifier into a fatal
* coverage gap, so a false positive fails the release.
*/
const CSS_IMPORT_RE = /import[^'";]*['"]([^'"]+\.css)['"]/g;
const CSS_IMPORT_RE = /\bimport\b(?!\s*\.)[^'";]*['"]([^'"]+\.css)['"]/g;
Comment thread
kojiwakayama marked this conversation as resolved.

/**
* Blank out regions that look like code but are not.
*
* A specifier found here is not merely ignored downstream: the release-asset
* build records anything it cannot resolve as a coverage gap, and gaps abort
* the build. So a commented-out import to a since-deleted stylesheet would
* block a project's releases permanently. Fenced blocks come first because
* they are delimited by the same backticks as template literals.
*
* Replaced with equivalent-length blanks rather than removed so any offset a
* caller derives from the result still lines up with the original source.
*/
function blankNonCodeRegions(source: string): string {
const blank = (match: string) => match.replace(/[^\n]/g, " ");
return source
.replace(/```[\s\S]*?```/g, blank)
.replace(/\/\*[\s\S]*?\*\//g, blank)
// Leading `[^:]` keeps the `//` in a URL like https://example.com from
// being read as the start of a comment.
.replace(
/(^|[^:])(\/\/[^\n]*)/g,
(_m, prefix: string, comment: string) => prefix + blank(comment),
)
.replace(/`(?:[^`\\]|\\[\s\S])*`/g, blank);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

/** Extract the raw specifiers of all static CSS imports in a source file. */
export function extractCssImportSpecifiers(source: string): string[] {
const specifiers: string[] = [];
for (const match of source.matchAll(CSS_IMPORT_RE)) {
for (const match of blankNonCodeRegions(source).matchAll(CSS_IMPORT_RE)) {
if (match[1]) specifiers.push(match[1]);
}
return specifiers;
Expand Down
51 changes: 51 additions & 0 deletions src/release-assets/build-executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,57 @@ export default defineConfig({ react: { version: "19.2.1" } });`,
);
});

it("merges module CSS from sources containing real JSX", async () => {
// Regression: the CSS import scan fed project source to es-module-lexer,
// which parses neither JSX nor TypeScript. Every .tsx file with a tag threw,
// each throw recorded a coverage gap, and gaps are fatal, so no project with
// a JSX component could publish a release.
//
// The suite missed it because every fixture put plain JavaScript inside
// .tsx files. These bodies are the shapes that actually broke in production:
// a closing component tag, a self-closing tag, and a nested element.
const rec: Recorded = { began: false, uploads: [], manifest: null, states: [] };
const files = [
{ path: "globals.css", content: ":root { --brand: blue; }" },
{ path: "app/styles.css", content: ".calc { background: #191919; }" },
{
path: "app/layout.tsx",
content: 'import "./styles.css";\n' +
"export default ({ children }) => (\n" +
" <html><head><title>Assistant</title></head><body>{children}</body></html>\n" +
");",
},
{
path: "app/markdown-renderer.tsx",
content: 'import ReactMarkdown from "react-markdown";\n' +
"export const R = ({ source }) => <ReactMarkdown>{source}</ReactMarkdown>;",
},
{
path: "pages/index.tsx",
content: 'import { Chat } from "veryfront/chat";\n' +
"export default () => <Provider><Chat /></Provider>;",
},
];
let seenStylesheet: string | undefined;
const client = makeClient(files, rec, {
compileProjectCss: (_candidates, stylesheet) => {
seenStylesheet = stylesheet;
return Promise.resolve(compiledCss(".calc{background:#191919}"));
},
});
const transform = () => Promise.resolve("export default null;");

// The build completing at all is the assertion that matters: a parse gap
// here aborts it with "Release asset coverage is incomplete".
await runReleaseAssetBuild(baseInput(client, transform), await tmp());

assertExists(seenStylesheet);
assert(
seenStylesheet!.includes(".calc"),
"CSS imported from a JSX-bearing layout must still be merged",
);
});

it("does not duplicate the resolved stylesheet when a module imports it directly", async () => {
const rec: Recorded = { began: false, uploads: [], manifest: null, states: [] };
const files = [
Expand Down
27 changes: 12 additions & 15 deletions src/release-assets/build-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import { FRAMEWORK_CANDIDATES } from "#veryfront/server/handlers/dev/framework-c
import { validateLexicalPath } from "#veryfront/security/path-validation.ts";
import {
CSS_IMPORTING_SOURCE_EXTENSIONS,
extractCssImportSpecifiers,
resolveCssImportPath,
} from "#veryfront/html/styles-builder/css-import-extraction.ts";
import { rewriteCssModuleContent } from "#veryfront/transforms/css-modules/naming.ts";
Expand Down Expand Up @@ -2915,21 +2916,17 @@ async function mergeModuleCssImports(
const importedPaths = new Set<string>();
for (const [path, content] of sourceByPath) {
if (!CSS_IMPORTING_SOURCE_EXTENSIONS.some((ext) => path.endsWith(ext))) continue;
let imports: Awaited<ReturnType<typeof parseImports>>;
try {
imports = await parseImports(content);
} catch (error) {
pushGap(gaps, `stylesheet-import-parse-failed:${path}`);
logger.warn("CSS import parsing failed during release asset build", {
path,
error: sanitizeError(error),
});
continue;
}

for (const imp of imports) {
const specifier = imp.n;
if (!specifier) continue;
// Regex extraction, not the ESM lexer. This runs over project source --
// .tsx/.jsx/.mdx/.ts by definition, see CSS_IMPORTING_SOURCE_EXTENSIONS --
// and es-module-lexer parses none of those. Every file containing JSX threw
// here, each throw recorded a gap, and gaps are fatal since #3244, so no
// project with a JSX component could publish a release at all.
//
// This is the same extractor the dev CSS scanner has always used, so the
// two paths now agree. It also removes the failure mode rather than
// handling it: a scanner that cannot throw cannot fail a release for a
// reason that has nothing to do with the release.
for (const specifier of extractCssImportSpecifiers(content)) {
Comment thread
kojiwakayama marked this conversation as resolved.
const cssPath = specifier.split(/[?#]/, 1)[0] ?? "";
if (!cssPath.endsWith(".css")) continue;
if (cssPath !== specifier) {
Expand Down
Loading