Conversation
🦋 Changeset detectedLatest commit: ece2dcc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for astro-starlight ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
bot
added
🚨 action
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
Small note (mostly for me to not forget) that tsdown 0.17.3 released last week added glob support to the |
`({ root?: ... } & { [x: string]: LocaleUserConfig })` is no longer assignable to `{ root?: ...; [x: string]: LocaleUserConfig }`
| 'The writing direction of this language; `"ltr"` for left-to-right (the default) or `"rtl"` for right-to-left.' | ||
| ), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Only adding a note for later as I'm not on my computer right now and to not forget, but it just hit me that these describe() have probably been removed when updating to Zod 4 and re-introduced when updating the PR.
| @@ -1,4 +1,4 @@ | |||
| import { FileIcons } from '../user-components/file-tree-icons'; | |||
| import { FileIcons, type FileIcon } from '../user-components/file-tree-icons'; | |||
|
|
|||
| export const BuiltInIcons = { | |||
There was a problem hiding this comment.
Note/thought: is it safe for us to do this?
| export const BuiltInIcons = { | |
| export const BuiltInIcons: Record<BuiltInIcon, string> = { |
The current test that checks the types align did not provide very readable output while I was resolving the merge — it’s almost impossible to tell from it which keys are missing:
packages/starlight/__tests__/basics/icon.test-d.ts:6:58 - error ts(2344): Type 'BuiltInIcon' does not satisfy the constraint '"Expected: literal string: download, Actual: literal string: firefox" | "Expected: literal string: download, Actual: literal string: chrome" | "Expected: literal string: download, Actual: literal string: edge" | ... 420 more ... | "Expected: literal string: substack, Actual: literal string: safari"'.
Type '"error"' is not assignable to type '"Expected: literal string: download, Actual: literal string: firefox" | "Expected: literal string: download, Actual: literal string: chrome" | "Expected: literal string: download, Actual: literal string: edge" | ... 420 more ... | "Expected: literal string: substack, Actual: literal string: safari"'.
6 expectTypeOf<keyof typeof BuiltInIcons>().toEqualTypeOf<BuiltInIcon>();
~~~~~~~~~~~
Whereas with an explicit Record it’s easy to identify missing keys.
I would need to test if this causes issues with inlining the union type as noted later in this file.
3 participants
Description
This PR is an early draft (not yet ready for review) that transpiles Starlight packages from TypeScript to JavaScript to avoid publishing TypeScript which is not a recommended or even supported practice.
Approach
Multiples approaches are usually available when it comes to transpiling in monorepos and keeping the flexibility of working with TypeScript source code in development.
Custom export condition was one of the first options considered, and would probably be the most long-term solution, but we cannot currently use them for Starlight. For the
docs/site to properly consume custom conditions in development, in Astro, such conditions would need to be defined in the Astro configuration using theviteproperty, altho Starlight itself is imported in such configuration so it's a chicken-and-egg problem right now. There is a world in the future where we would be able to run Astro usingnode --conditionsdirectly, and Vite automatically picks up those conditions, but this requires some changes in Node that are not yet available.Considering the above, this PR relies on
pnpmand specifically on the fact thatpnpmautomatically overrides some fields inpackage.jsonfiles when packages are published withpnpm publishby the one defined in thepackage.jsonpublishConfigproperty. In the case of Starlight, we maintain 2exportsfields: one for development (pointing to the TypeScript source files like it is today) and one for production (pointing to the transpiled JavaScript files).How to test
When getting closer to a reviewable state, some pre-release packages will probably be published to test the changes in a real-world scenario.
In the meantime, you can test the changes locally using a different Starlight project outside of the monorepo, e.g. one setup using
pnpm create astro --template starlight.Remaining tasks
pnpm changeset publishusespnpm publishunder the hood (it probably does, aspackage-manager-detectoris used when actually publishing to figure out the package manager but need to double check)