refactor(astro): replace tsconfck with get-tsconfig#16433
Conversation
|
Merging this PR will improve performance by 10.87%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | Build: hybrid site (static + server) |
8.8 s | 7.9 s | +10.87% |
Comparing ocavue-forks:ocavue/get-tsconfig-4 (b6e1db9) with main (b7916ef)1
Footnotes
485c17f to
87dd769
Compare
d9ddf6f to
a2c55af
Compare
| compilerOptions?: StripEnums<CompilerOptions>; | ||
| compileOnSave?: boolean; | ||
| extends?: string; | ||
| extends?: string | string[]; |
There was a problem hiding this comment.
Note for reviewers:
extends can be an array since TS 5.0
| */ | ||
| export async function loadTSConfig( | ||
| root: string | undefined, | ||
| findUp = false, |
There was a problem hiding this comment.
Note for reviewers:
findUp is never passed; it was always false. I removed findUp in this PR.
| tsconfigFile: normalize(resolved.path), | ||
| tsconfig: resolved.config satisfies TSConfig, | ||
| rawConfig, | ||
| sources: (resolved.sources || [resolved.path]).map(normalize), |
There was a problem hiding this comment.
Note for reviewers:
Here I use normalize to return the backward slash \ on Windows. This aligns with the previous behavior. I don't see any issue with returning / on Windows, but it's good to be cautious just in case.
75707a9 to
ceb6749
Compare
ceb6749 to
330a927
Compare
There was a problem hiding this comment.
If there's no user facing change, we can skip the changeset
There was a problem hiding this comment.
deleted the changeset file
delucis
left a comment
There was a problem hiding this comment.
@ocavue Did you investigate the option of using the rolldown utilities as suggested in the tsconfck issue: dominikg/tsconfck#240? The issue links Vite’s PR where they did this: vitejs/vite#21577.
That would have to be against next where we have Vite 8 + Rolldown available but could be worth doing to avoid the extra dep. (If it covers everything we need — I haven’t looked yet, but I had it on my todo list.)
|
That's also on my todo list. I will check it out. |
|
I have two concerns about reusing this API in Astro:
|
There was a problem hiding this comment.
IA with #16433 (comment) I feel IMHO this change should be blocked until there is a clear more direction from the vite team.
## Summary - resolves TypeScript from 5.9.2 to 5.9.3 in `pnpm-lock.yaml` - keeps the existing `package.json` range on TypeScript 5 so Astro tooling remains inside its declared peer dependency support ## Why not TypeScript 6 yet? TypeScript 6 is the npm latest, but it is a real migration release rather than a routine patch update. The TypeScript 6 notes call out changed compiler defaults, root directory behavior, stricter side-effect import handling, deprecated options, and other migration concerns. Astro support is also still catching up: - withastro/astro#16112 tracks `@astrojs/check@0.9.8` still requiring `typescript@^5.0.0`. - withastro/astro#16385 tracks Astro still depending on `tsconfck`, which also declares `typescript@^5.0.0` and is being discontinued. - withastro/astro#16433 is the open upstream PR to replace `tsconfck` with `get-tsconfig`; it is not merged yet. Keeping this PR to TypeScript 5.9.3 gets the latest supported 5.x patch without opting into peer dependency overrides or upstream compatibility assumptions. ## Validation - `mise exec -- corepack pnpm install --frozen-lockfile` - `mise exec -- corepack pnpm test -- --run` - `mise exec -- corepack pnpm format:check` - `mise run build`
|
i think it makes sense for vite to expose it and started a feature request here |
|
I will resolve the conflict |
This auto block is no longer relevant as this is not a minor change anymore.
|
Awesome, thank you @ocavue Given that today we have a release, let's wait to merge it until tomorrow. |
|
@ematipico Thanks! @delucis After this commit 696018c, the situation with fake tests is significantly improving (though not entirely resolved). Check out the CI status records in the screenshot below:
|
- Add missing dependencies (get-tsconfig, jsonc-parser) and remove tsconfck (replaced on main by PR #16433 but deps were lost during merge) - Remove unused replaceTopLevelReturns export from vite-plugin-astro/utils.ts (function from main's PR #16552 not used by next's Rust compiler, fixes knip lint) - Update lightningcss-scoped-nesting test expectations for Rust compiler (next branch handles CSS scoping differently, test from PR #16548) - Format compile.ts (remove extra blank line)
* Fix merge-fix workflow to handle conflict markers before install (#16539) * Handle merge conflicts in merge-fix workflow by stripping JSON/YAML markers and verifying via AI (#16541) * [ci] format * fix(render): avoid script dedup state consumption in inert template c… (#16527) * fix(render): avoid script dedup state consumption in inert template contexts #16525 * fix(render): normalize inert script dedup test outputs to strings * fix: add missing hydration metadata fields in inert-script-dedup tests * test(app): add component-level inert template script dedup coverage * test(app): stabilize inert script dedup regression coverage * fix(astro): move inert template dedup logic into hydration/directive script guards * [ci] format * fix: emit fallback rewrite pages (#16515) * fix(i18n): emit fallback rewrite pages * chore: add changeset * [ci] format * refactor(astro): replace tsconfck with get-tsconfig (#16433) * fix(language-server): remove circular dependency on svelte/vue integrations (#16532) * [ci] format * fix(upgrade): use bundled JS output (#16547) * Update dependency postcss to v8.5.10 [SECURITY] (#16489) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update dependency fastify to v5.8.5 [SECURITY] (#16346) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update dependency fast-xml-parser to v5.7.0 [SECURITY] (#16452) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * fix(cloudflare): preserve existing KV namespace bindings when injecting SESSION (#16555) * fix(slots): unwrap conditional slot callbacks (#16509) Co-authored-by: cyphercodes <cyphercodes@users.noreply.github.com> * fix: preserve CSS propagation for partial pages imported as components (#16415) * fix(astro): preserve CSS from imported partial pages Treat top-level pages as CSS boundaries only when they are referenced exclusively by the virtual page module, so partial pages imported as components continue propagating transitive styles in production builds. Made-with: Cursor * test(astro): fix partial CSS fixture component import path Correct the regression fixture path so the partial page can resolve ResultsTable during build on CI. Made-with: Cursor * test(astro): make partial css assertion robust to minification Assert that scoped ResultsTable selectors are present in extracted styles instead of matching a specific color token that can be minified differently across environments. Made-with: Cursor * chore: rerun CI after unrelated e2e flake Trigger a fresh workflow run for PR #16415 after unrelated flaky E2E failures in actions-blog/cloudflare/hmr tests. Made-with: Cursor * fix: propagate head metadata across ssr and prerender envs in dev (#16292) * fix: propagate head metadata across ssr and prerender envs in dev Signed-off-by: Patrick Linnane <patrick@linnane.io> * test(astro): cover head propagation through prerender env Signed-off-by: Patrick Linnane <patrick@linnane.io> --------- Signed-off-by: Patrick Linnane <patrick@linnane.io> * fix(assets): pass through images Sharp cannot decode instead of crashing (#16451) * fix(assets): pass through images Sharp cannot decode instead of crashing * test(assets): add test for animated AVIF pass-through * test(assets): add test for animated AVIF pass-through * fix(test): remove incorrect avif extension assertion * chore: add changeset for animated AVIF fix * feat(assets): add warning when Sharp cannot optimize unsupported image formats * chore: lint and whitespace * [ci] format * Update dependency @fastify/middie to v9.3.2 [SECURITY] (#16369) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * perf(astro): skip session storage lookup if no cookie is set (#16540) * Fix defineLiveCollection interface loader typing (#16018) * Fix defineLiveCollection interface loader typing * Retrigger CI * Retrigger CI * [ci] format * Fix style compilation failure when importing components via tsconfig path aliases (#15994) * fix: resolve relative virtual module IDs in normalizeFilename for path alias styles When a component with a <style> tag is imported via a TypeScript path alias, certain environments (Windows + newer Node.js) can produce a relative virtual module ID (e.g. ./src/components/Foo.astro?astro&type=style). The load handler passed this through normalizeFilename to look up the compile metadata cache, but the function had no branch for ./-prefixed relative paths, so it returned the path unchanged instead of resolving it to an absolute path. This caused a cache miss and a build error. Adds a branch to resolve relative paths against root, matching the behavior used for /@fs and /path variants. Fixes #15963 * test: assert against emitted CSS file for path alias style regression test * fix(cloudflare): fix static assets and prerendered pages 404ing when `base` is configured. (#16277) * fix(cloudflare): static assets being in the wrong place * test(cloudflare): add tests * add changeset --------- Co-authored-by: Matthew Phillips <matthew@matthewphillips.info> * [ci] format * fix(test): update test-utils import from .js to .ts in .test.js files (#16560) * fix(frontmatter): preserve strings and comments when replacing top-le… (#16552) * fix(frontmatter): preserve strings and comments when replacing top-level return (#16551) * chore: add changeset for cloudflare KV namespace fix * fix(cloudflare): re-apply KV namespace merge fix after source revert --------- Co-authored-by: Matthew Phillips <matthewphillips@cloudflare.com> * fix: processing procedure for dynamic paths (#16144) * feat: test maching dynamic routes contain .html in their params * fix: processing procedure for dynamic paths * feat: changeset file for processing procedure for dynamic pahts * refactor(routing): move .html stripping logic from getParams to getProps * fix: UT of getParams function by fixing path mismatch error returning html file * fix: logic of removing .html extention * Apply suggestion from @Fryuni Co-authored-by: Luiz Ferraz <luiz@lferraz.com> * Apply suggestion from @Fryuni Co-authored-by: Luiz Ferraz <luiz@lferraz.com> * Apply suggestion from @Fryuni Co-authored-by: Luiz Ferraz <luiz@lferraz.com> --------- Co-authored-by: Matthew Phillips <matthew@matthewphillips.info> Co-authored-by: Luiz Ferraz <luiz@lferraz.com> Co-authored-by: Matthew Phillips <matthewphillips@cloudflare.com> * fix(css): preserve scope on nested & with lightningcss transformer (#16548) * fix(css): preserve scope on nested & with lightningcss transformer with `vite.css.transformer: 'lightningcss'`, scoped styles using `:where(& > ...)` (the shape tailwind v4's `space-x-*`, `space-y-*`, and `divide-*` expand to) put the scope attribute on the matched child instead of the parent. lightningcss flattened the nesting before `@astrojs/compiler` ran scope injection, so by then `:where(...)` was the leading compound and the injector prepended `[data-astro-cid-X]` as a new leading compound — constraining the wrong element. ask lightningcss to skip nesting lowering during the per-component `preprocessCSS` call (via `Features.Nesting` on a shallow-cloned config — non-mutating, safe under parallel compiles). vite's final pipeline still lowers nesting for the bundle. adds a regression test covering the reporter's exact shape. Fixes #16524 * docs(changeset): rewrite per astro guidelines --------- Co-authored-by: Matthew Phillips <matthewphillips@cloudflare.com> * Fixes @_@ not being stripped from CSS file names (#16264) Co-authored-by: Matthew Phillips <matthew@matthewphillips.info> Co-authored-by: Matthew Phillips <matthewphillips@cloudflare.com> * fix : astro image position prop bug (#16236) * feat: test of astro image position prop bug * fix: logic of getItem function * feat: changeset file * fix: create data-astro-image-pos attribute all time for path csp unit test * feat: another test of getImage function * fix: change set explanetion more clealy for users * Add: yaml file * chore: clean up changeset --------- Co-authored-by: Matthew Phillips <matthew@matthewphillips.info> Co-authored-by: Matthew Phillips <matthewphillips@cloudflare.com> * [ci] format * fix(build): exclude prerendered route styles from SSR manifest (#16517) Inline CSS for prerendered routes is dead weight in the SSR manifest: the prerendered HTML on disk already contains the <style> tags, and the SSR worker never renders these routes. Strip styles for prerendered routes from the SSR manifest while leaving the prerender manifest (and prerendered HTML) unchanged. Cuts SSR entry-chunk size on hybrid builds with build.inlineStylesheets: "always", reducing cold-start parse cost on Cloudflare Workers and similar platforms. * fix(prefetch): trigger tap strategy when clicking nested child elements (#16566) * fix(prefetch): trigger tap strategy when clicking nested child elements * fix(prefetch): trigger tap and hover strategies when clicking nested child elements * fix(astro): persist session delete without prior get/set (#16565) * fix(astro): persist session delete without prior get/set * est(e2e): sync collectLoads to avoid race with assertions * fix(astro): persist session delete without prior get/set * fix: route mismatch using trail slash never (#16516) * feat: test for route match fails when trailingSlash never * fix: root route matching with trailingSlash: 'never' and base path * feat: changeset file * fix: root route matching with trailingSlash 'never' and base path in dev server and rewrites The `normalizeRewritePathname` in rewrite.ts was returning '' (empty string) for root-path rewrites with a non-root base, and app.ts was converting the stripped pathname '/' to '' before route matching. Both relied on the old '^$' pattern for the index route. After the pattern.ts fix made the root pattern '^/$', these normalizations broke matching. Remove the '/' -> '' conversions so route.pattern.test('/') correctly matches the fixed '^/$' pattern in both direct requests and Astro.rewrite() calls * fix: add regression test for root path normalization with base --------- * [ci] format * [ci] release (#16545) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix: resolve merge conflicts from main into next * fix: resolve merge issues from main into next - Add missing dependencies (get-tsconfig, jsonc-parser) and remove tsconfck (replaced on main by PR #16433 but deps were lost during merge) - Remove unused replaceTopLevelReturns export from vite-plugin-astro/utils.ts (function from main's PR #16552 not used by next's Rust compiler, fixes knip lint) - Update lightningcss-scoped-nesting test expectations for Rust compiler (next branch handles CSS scoping differently, test from PR #16548) - Format compile.ts (remove extra blank line) * chore: trigger CI * fix: add head-inject directive to propagated assets module for CSS injection * chore: update main-to-next merge * chore: retrigger CI * chore: retrigger CI --------- Signed-off-by: Patrick Linnane <patrick@linnane.io> Co-authored-by: Matthew Phillips <matthewphillips@cloudflare.com> Co-authored-by: Matthew Phillips <matthewp@users.noreply.github.com> Co-authored-by: Chan <101856681+enjoyandlove@users.noreply.github.com> Co-authored-by: knj <kenji.tomita1996@gmail.com> Co-authored-by: knj <ematipico@users.noreply.github.com> Co-authored-by: ocavue <ocavue@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: web-dev0521 <jasonpette1783@gmail.com> Co-authored-by: Rayan Salhab <r.salhab@aiyexpertsolutions.com> Co-authored-by: cyphercodes <cyphercodes@users.noreply.github.com> Co-authored-by: 0x K. <66915025+0xbejaxer@users.noreply.github.com> Co-authored-by: Patrick Linnane <patrick@linnane.io> Co-authored-by: Maxim Slobodchikov <93232189+maximslo@users.noreply.github.com> Co-authored-by: Matt Kane <m@mk.gg> Co-authored-by: Felmon <felmonon@gmail.com> Co-authored-by: Ossaid <imossaidquadri@gmail.com> Co-authored-by: Calvin Liang <me@calvin.sh> Co-authored-by: Matthew Phillips <matthew@matthewphillips.info> Co-authored-by: fkatsuhiro <113022468+fkatsuhiro@users.noreply.github.com> Co-authored-by: Luiz Ferraz <luiz@lferraz.com> Co-authored-by: Utpal Sen <utpalsen902@gmail.com> Co-authored-by: atsbob <98831687+atsbob@users.noreply.github.com> Co-authored-by: Adam Chalemian <adam@chal.net> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Summary
Replaces the discontinued
tsconfckdependency withget-tsconfig@5.0.0-beta.4andjsonc-parser. This is a step forward to support TS 6.0 astsconfckdoesn't support TS 6 in its peerDep.Closes #16385
Why beta
I need to use
get-tsconfigv5 beta instead of the v4 stable becauseget-tsconfigv4 leaks a feature that resolves all extended file paths for the purpose of dev server watch files (see privatenumber/get-tsconfig#119).If astro team doesn't want to ship a beta dependency, we can wait for the
get-tsconfigv5 stable.What's changed
jsonc-parserto parse the raw tsconfig fileget-tsconfigto resolve the tsconfig (i.e., mergeextends) and get a list of all files fromextends.'unknown-error'error typeNotes for maintainers
get-tsconfigis pinned to exactly5.0.0-beta.4(no caret).jsonc-parseris added as a direct dep. It was already transitively present via Vite, so no new bytes on disk.Test
Docs
Changeset added.