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

fix: handle build.format links in sidebar #1023

Merged
merged 49 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
09991e7
handle file build format
Nov 2, 2023
07374b1
test with docs - remove this
Nov 2, 2023
15bd1fb
add build to test config
Nov 2, 2023
48cd6f9
add tests
Nov 2, 2023
6e5cff1
rename config
Nov 2, 2023
4a33f7f
fix typo and ignore absolute paths
Nov 2, 2023
38d21c0
small rfc for external links
Nov 2, 2023
de47d27
Apply Chris suggestions
kevinzunigacuellar Nov 2, 2023
12153cf
reuse defineVitestConfig
Nov 2, 2023
c0e0b1b
Apply Chris suggestions
kevinzunigacuellar Nov 2, 2023
d5bb8cb
make ts happy
Nov 2, 2023
1d10e4e
add formatPath function with tests
Nov 3, 2023
858c073
add formatpath to SiteTitle
Nov 3, 2023
b13888e
remove assertion
Nov 3, 2023
c04e4dd
add test for pathformat
Nov 3, 2023
8dd08c7
add format to navigation
Nov 3, 2023
2b8ac77
Fix formatPath utility function and update imports
Nov 6, 2023
bfa0c4d
Update formatPath function in SiteTitle component
Nov 6, 2023
8a5607b
Remove docs changes
Nov 6, 2023
a2de86b
format currentPath and move utils to format-path
Nov 6, 2023
84d2871
forward project config to SiteTitle
Nov 6, 2023
18b614c
combine imports
Nov 6, 2023
42239f6
Centralise all path utilities in `utils/path.ts`
delucis Nov 16, 2023
d08e12a
Simplify `ensureHtmlExtension()`
delucis Nov 16, 2023
d5ffe1e
Distinguish content collection ID stripping from `.html` path normali…
delucis Nov 16, 2023
0e07f5f
Enable `trailingSlash: 'always'` in the Starlight docs
delucis Nov 16, 2023
40bb976
Skip trailing slash handling for `build.format: 'file'` in `formatPat…
delucis Nov 16, 2023
083c47d
Ensure `/` for root paths
delucis Nov 16, 2023
55e00dd
Update snapshots now that we actually use default of `trailingSlash: …
delucis Nov 16, 2023
1f491e3
refactor test: Use `test.each()` instead of `for..of` in `test()`
delucis Nov 16, 2023
8a77650
Update test to reflect new default of `trailingSlash: 'ignore'`
delucis Nov 16, 2023
a8cc600
Make base utils unopinionated about trailing slashes
delucis Nov 16, 2023
474cb1e
Preserve trailing slashes when stripping HTML extensions
delucis Nov 16, 2023
3003309
Update tests
delucis Nov 16, 2023
5f10cda
Add `trailingSlash` to project context virtual module
delucis Nov 16, 2023
90ccde4
Pass `trailingSlash` option when calling `formatPath()`
delucis Nov 16, 2023
9322cb2
Fix prev/next links showing `isCurrent: true`
delucis Nov 16, 2023
6585b56
Fix testing for `isCurrent`
delucis Nov 16, 2023
a7e24b1
Use `formatPath()` for all sidebar links
delucis Nov 16, 2023
00fc228
Fix test
delucis Nov 16, 2023
8ab28e6
Refactor `formatPath()` so core call sites don’t need to pass config
delucis Nov 16, 2023
606ee1e
Merge branch 'main' into kev-build-format
delucis Nov 16, 2023
ed66d52
Clean up test
delucis Nov 16, 2023
f95ee7e
Don’t double format sidebar links (would add `base` twice)
delucis Nov 16, 2023
7192d95
Add changeset
delucis Nov 16, 2023
e1c3a28
Merge branch 'main' into kev-build-format
delucis Nov 17, 2023
578e9dc
Fix snapshot
delucis Nov 17, 2023
06e499f
Merge branch 'main' into kev-build-format
delucis Nov 17, 2023
596d889
Remove unused assignment
delucis Nov 17, 2023
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
25 changes: 25 additions & 0 deletions .changeset/sharp-tables-march.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
'@astrojs/starlight': minor
---

Respect the `trailingSlash` and `build.format` Astro options when creating Starlight navigation links.

⚠️ **Potentially breaking change:**
This change will cause small changes in link formatting for most sites.
These are unlikely to break anything, but if you care about link formatting, you may want to change some Astro settings.

If you want to preserve Starlight’s previous behavior, set `trailingSlash: 'always'` in your `astro.config.mjs`:

```js
import { defineConfig } from 'astro/config';
import starlight from '@astrojs/starlight';

export default defineConfig({
trailingSlash: 'always',
integrations: [
starlight({
// ...
}),
],
});
```
1 change: 1 addition & 0 deletions docs/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const site = VERCEL_PREVIEW_SITE || 'https://starlight.astro.build/';

export default defineConfig({
site,
trailingSlash: 'always',
integrations: [
starlight({
title: 'Starlight',
Expand Down
4 changes: 2 additions & 2 deletions packages/starlight/__tests__/basics/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ describe('pathWithBase()', () => {
test('does not prepend anything', () => {
expect(pathWithBase('/path/')).toBe('/path/');
});
test('adds leading and trailing slashes if needed', () => {
expect(pathWithBase('path')).toBe('/path/');
test('adds leading slash if needed', () => {
expect(pathWithBase('path')).toBe('/path');
});
});

Expand Down
105 changes: 105 additions & 0 deletions packages/starlight/__tests__/basics/format-path.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import { describe, expect, test } from 'vitest';
import { createPathFormatter } from '../../utils/createPathFormatter';

type FormatPathOptions = Parameters<typeof createPathFormatter>[0];
const formatPath = (href: string, opts: FormatPathOptions) => createPathFormatter(opts)(href);

describe.each<{ options: FormatPathOptions; tests: Array<{ path: string; expected: string }> }>([
{
options: { format: 'file', trailingSlash: 'ignore' },
tests: [
// index page
{ path: '/', expected: '/index.html' },
// with trailing slash
{ path: '/reference/configuration/', expected: '/reference/configuration.html' },
// without trailing slash
{ path: '/api/v1/users', expected: '/api/v1/users.html' },
// with file extension
{ path: '/guides/components.html', expected: '/guides/components.html' },
// with file extension and trailing slash
{ path: '/guides/components.html/', expected: '/guides/components.html' },
],
},
{
options: { format: 'file', trailingSlash: 'always' },
tests: [
// index page
{ path: '/', expected: '/index.html' },
// with trailing slash
{ path: '/reference/configuration/', expected: '/reference/configuration.html' },
// without trailing slash
{ path: '/api/v1/users', expected: '/api/v1/users.html' },
// with file extension
{ path: '/guides/components.html', expected: '/guides/components.html' },
// with file extension and trailing slash
{ path: '/guides/components.html/', expected: '/guides/components.html' },
],
},
{
options: { format: 'file', trailingSlash: 'never' },
tests: [
// index page
{ path: '/', expected: '/index.html' },
// with trailing slash
{ path: '/reference/configuration/', expected: '/reference/configuration.html' },
// without trailing slash
{ path: '/api/v1/users', expected: '/api/v1/users.html' },
// with file extension
{ path: '/guides/components.html', expected: '/guides/components.html' },
// with file extension and trailing slash
{ path: '/guides/components.html/', expected: '/guides/components.html' },
],
},
{
options: { format: 'directory', trailingSlash: 'always' },
tests: [
// index page
{ path: '/', expected: '/' },
// with trailing slash
{ path: '/reference/configuration/', expected: '/reference/configuration/' },
// without trailing slash
{ path: '/api/v1/users', expected: '/api/v1/users/' },
// with file extension
{ path: '/guides/components.html', expected: '/guides/components/' },
// with file extension and trailing slash
{ path: '/guides/components.html/', expected: '/guides/components/' },
],
},
{
options: { format: 'directory', trailingSlash: 'never' },
tests: [
// index page
{ path: '/', expected: '/' },
// with trailing slash
{ path: '/reference/configuration/', expected: '/reference/configuration' },
// without trailing slash
{ path: '/api/v1/users', expected: '/api/v1/users' },
// with file extension
{ path: '/guides/components.html', expected: '/guides/components' },
// with file extension and trailing slash
{ path: '/guides/components.html/', expected: '/guides/components' },
],
},
{
options: { format: 'directory', trailingSlash: 'ignore' },
tests: [
// index page
{ path: '/', expected: '/' },
// with trailing slash
{ path: '/reference/configuration/', expected: '/reference/configuration/' },
// without trailing slash
{ path: '/api/v1/users', expected: '/api/v1/users' },
// with file extension
{ path: '/guides/components.html', expected: '/guides/components' },
// with file extension and trailing slash
{ path: '/guides/components.html/', expected: '/guides/components' },
delucis marked this conversation as resolved.
Show resolved Hide resolved
],
},
])(
'formatPath() with { format: $options.format, trailingSlash: $options.trailingSlash }',
({ options, tests }) => {
delucis marked this conversation as resolved.
Show resolved Hide resolved
test.each(tests)('returns $expected for $path', ({ path, expected }) => {
expect(formatPath(path, options)).toBe(expected);
});
}
);
27 changes: 19 additions & 8 deletions packages/starlight/__tests__/basics/navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,34 @@ describe('getSidebar', () => {
`);
});

test('marks current path with isCurrent', () => {
const paths = ['/', '/environmental-impact/', '/guides/authoring-content/'];
for (const currentPath of paths) {
test.each(['/', '/environmental-impact/', '/guides/authoring-content/'])(
'marks current path with isCurrent: %s',
(currentPath) => {
const items = flattenSidebar(getSidebar(currentPath, undefined));
const currentItems = items.filter((item) => item.type === 'link' && item.isCurrent);
expect(currentItems).toHaveLength(1);
const currentItem = currentItems[0];
if (currentItem?.type !== 'link') throw new Error('Expected current item to be link');
expect(currentItem.href).toBe(currentPath);
}
});
);

test('ignore trailing slashes when marking current path with isCurrent', () => {
const pathWithoutTrailingSlash = '/environmental-impact';
const items = flattenSidebar(getSidebar(pathWithoutTrailingSlash, undefined));
const pathWithTrailingSlash = '/environmental-impact/';
const items = flattenSidebar(getSidebar(pathWithTrailingSlash, undefined));
const currentItems = items.filter((item) => item.type === 'link' && item.isCurrent);
expect(currentItems).toMatchObject([{ href: `${pathWithoutTrailingSlash}/`, type: 'link' }]);
expect(currentItems).toMatchInlineSnapshot(`
[
{
"attrs": {},
"badge": undefined,
"href": "/environmental-impact/",
"isCurrent": true,
"label": "Eco-friendly docs",
"type": "link",
},
]
`);
});

test('nests files in subdirectory in group when autogenerating', () => {
Expand Down Expand Up @@ -243,7 +254,7 @@ describe('getPrevNextLinks', () => {
expect(withDefaults.prev).toBeUndefined();
expect(withCustomLinks.prev).toEqual({
type: 'link',
href: '/x/',
href: '/x',
label: 'X',
isCurrent: false,
attrs: {},
Expand Down
125 changes: 125 additions & 0 deletions packages/starlight/__tests__/build-format-file/navigation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import { describe, expect, test, vi } from 'vitest';
import { getSidebar } from '../../utils/navigation';

vi.mock('astro:content', async () =>
(await import('../test-utils')).mockedAstroContent({
docs: [
['index.mdx', { title: 'Home Page' }],
['environmental-impact.md', { title: 'Eco-friendly docs' }],
['reference/configuration.mdx', { title: 'Config Reference' }],
['reference/frontmatter.md', { title: 'Frontmatter Reference' }],
// @ts-expect-error — Using a slug not present in Starlight docs site
['api/v1/users.md', { title: 'Users API' }],
['guides/components.mdx', { title: 'Components' }],
],
})
);

describe('getSidebar with build.format = "file"', () => {
test('returns an array of sidebar entries with its file extension', () => {
expect(getSidebar('/', undefined)).toMatchInlineSnapshot(`
[
{
"attrs": {},
"badge": undefined,
"href": "/index.html",
"isCurrent": true,
"label": "Home",
"type": "link",
},
{
"badge": undefined,
"collapsed": false,
"entries": [
{
"attrs": {},
"badge": {
"text": "New",
"variant": "success",
},
"href": "/intro.html",
"isCurrent": false,
"label": "Introduction",
"type": "link",
},
{
"attrs": {},
"badge": {
"text": "Deprecated",
"variant": "default",
},
"href": "/next-steps.html",
"isCurrent": false,
"label": "Next Steps",
"type": "link",
},
{
"attrs": {
"class": "showcase-link",
"target": "_blank",
},
"badge": undefined,
"href": "/showcase.html",
"isCurrent": false,
"label": "Showcase",
"type": "link",
},
{
"attrs": {},
"badge": undefined,
"href": "https://astro.build/",
"isCurrent": false,
"label": "Astro",
"type": "link",
},
],
"label": "Start Here",
"type": "group",
},
{
"badge": {
"text": "Experimental",
"variant": "default",
},
"collapsed": false,
"entries": [
{
"attrs": {},
"badge": undefined,
"href": "/reference/configuration.html",
"isCurrent": false,
"label": "Config Reference",
"type": "link",
},
{
"attrs": {},
"badge": undefined,
"href": "/reference/frontmatter.html",
"isCurrent": false,
"label": "Frontmatter Reference",
"type": "link",
},
],
"label": "Reference",
"type": "group",
},
{
"badge": undefined,
"collapsed": false,
"entries": [
{
"attrs": {},
"badge": undefined,
"href": "/api/v1/users.html",
"isCurrent": false,
"label": "Users API",
"type": "link",
},
],
"label": "API v1",
"type": "group",
},
]
`);
});
});
51 changes: 51 additions & 0 deletions packages/starlight/__tests__/build-format-file/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { defineVitestConfig } from '../test-config';

export default defineVitestConfig(
{
title: 'Sidebar Test',
sidebar: [
// A single link item labelled “Home”.
{ label: 'Home', link: '/' },
// A group labelled “Start Here” containing two links.
{
label: 'Start Here',
items: [
{
label: 'Introduction',
link: '/intro',
badge: {
variant: 'success',
text: 'New',
},
},
{ label: 'Next Steps', link: '/next-steps', badge: 'Deprecated' },
{
label: 'Showcase',
link: '/showcase',
attrs: { class: 'showcase-link', target: '_blank' },
},
{
label: 'Astro',
link: 'https://astro.build/',
},
],
},
// A group linking to all pages in the reference directory.
{
label: 'Reference',
badge: 'Experimental',
autogenerate: { directory: 'reference' },
},
// A group linking to all pages in the api/v1 directory.
{
label: 'API v1',
autogenerate: { directory: '/api/v1/' },
},
],
},
{
build: {
format: 'file',
},
}
);
Loading
Loading