🦋 Changeset detected

Latest commit: 596d889

The changes in this PR will be included in the next version bump.

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!

Name	Link
🔨 Latest commit	18b614c
🔍 Latest deploy log	https://app.netlify.com/sites/astro-starlight/deploys/65497dc6d95bfa0008d6e75e
😎 Deploy Preview	https://deploy-preview-1023--astro-starlight.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 100 (no change from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 92 (no change from production) PWA: - View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

size-limit report 📦

I see build.format: 'file' in the docs to test, but looks like the sidebar in the deploy preview does not use .html. Any idea why?

Checking the build output it looks that all the href in the sidebar have a .html append to the end. I think it's one of netlify features to strip the .html from the deployed files (Pretty URL’s)

Checking the build output it looks that all the href in the sidebar have a .html append to the end. I think it's one of netlify features to strip the .html from the deployed files (Pretty URL’s)

Ahhhhh! Thank you Netlify 🖖

I had to make a few assertions for AstroConfig['build'] because Partial only makes the top level properties optional and it expects a fully defined build property to be passed to the user config plugin.

On second thought, we could create a default fully defined build config and override it with opts.

I am open to suggestions

I made a function called formatPathFromConfig that formats any given path using the astro config options (base and build.format). I am not sure where to put it for now its just on navigation utils. And I wrote a few tests with different edge cases. What do you think?

Nice work!

I am not sure where to put it for now its just on navigation utils.

Might make sense to move it to its own file. We do have path.ts and base.ts, but those both avoid relying on the virtual imports.

The other option would be to refactor it slightly to take the required config as an argument and put it together in path.ts:

/** Add `.html` extension to a path. */
export function ensureHtmlExtension(path: string) {
	if (path.endsWith('.html')) return ensureLeadingSlash(path);
	path = stripLeadingAndTrailingSlashes(path);
	return path ? ensureLeadingSlash(path) + '.html' : '/index.html';
}

export function formatPath(href: string, format: 'file' | 'directory') {
	// atach base
	href = format === 'file' ? fileWithBase(href) : pathWithBase(href);
	// add html extension
	href = format === 'file' ? ensureHtmlExtension(href) : stripExtension(href);
	return href;
}

Then users could import it and pass the format:

import project from 'virtual:starlight/project-context';
import { formatPath } from './path';

formatPath(path, project.build.format);

I've created a new file for the format function and added trailingSlash as an option, given its relevance to the project configuration.

There's some ambiguity regarding the expected behavior for trailingSlash: "ignore". Should we establish a convention similar to the existing one for adding trailing slashes to links? How might this convention apply when the build format is set to "file," or should we disregard it entirely?

In my view, adopting a convention would simplify the comparison of values for "isCurrent" in the sidebar. The convention I propose is as follows:

• format: "directory", trailingSlash: "ignore": Retain the trailing slash (similar to the existing setup).

  • "/reference" becomes "/reference/"
  • "/reference.html" becomes "/reference/"

• format: "file", trailingSlash: "ignore": Omit the trailing slash.

  • "/reference" becomes "/reference.html"
  • "/reference/" becomes "/reference.html"

Establishing this convention would enhance consistency and aid in comparing values for "isCurrent" in the sidebar, offering a clearer and more predictable structure for the project. What do you think? @delucis @HiDeoo

PS: When reviewing the test file for formatPath, kindly disregard the tests related to the trailing slash set to "ignore". I accidentally duplicated the expected values from other tests 😅. The rest of the test sets should be accurate and reliable.

Hey @kevinzunigacuellar! Turned out this was fiddlier than I expected, so I spent some time getting everything polished up.

Quick summary of the changes I made:

• Wire up a project’s trailingSlash option with the virtual module and apply it when formatting paths.
• Move around some of the logic about when slashes are added/removed to ensure that with trailingSlash: 'ignore' (the Astro default) we don’t modify trailing slashes on user-specified links.
• Make sure we never add a trailing slash when build.format is set to file — I’m pretty sure /example.html should never be /example.html/.
• Fixed checks for whether a sidebar link was for the current page by always formatting the comparison links with a formatter that sets trailingSlash: 'always' so they will be consistent.
• Made the formatPath() helper easier to use in Starlight code by exporting it already configured with the project settings. Formatters with different settings can be created with the createPathFormatter() factory method.

Would love a quick look if you have time and thanks for the patience waiting for me on this one!

Lost Glasses, Troublesome Moment! ✅