Skip to content

Commit

Permalink
[content collections] Handle file name spaces and capitalization (#5666)
Browse files Browse the repository at this point in the history
* feat: slugify slug to handle capitals and spaces

* docs: add not on Slugger issues

* deps: bump to github-slugger 2.0

* refactor: new Slugger() -> slug util

* fix: stop using URL.pathname

* fix: `file://` prefix on isContentFlagImport

* test: spaces in fixture file name

* chore: add `test:unit:match`

* refactor: handle collection errors from getEntryInfo

* test: unit getEntryInfo

* chore: changeset

* chore: markdown-remark out of date

* fix: correctly strip index on windows

* fix: move to utils, fix slug regex

* refactor: intermediate var

* lint: `path` variable shadowing

* chore: add not on allowFilesOutsideCollection
  • Loading branch information
bholmesdev committed Jan 3, 2023
1 parent 19be918 commit bf210f7
Show file tree
Hide file tree
Showing 11 changed files with 158 additions and 62 deletions.
7 changes: 7 additions & 0 deletions .changeset/stupid-shoes-complain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'astro': minor
---

Correctly handle spaces and capitalization in `src/content/` file names. This introduces github-slugger for slug generation to ensure slugs are usable by `getStaticPaths`. Changes:
- Resolve spaces and capitalization: `collection/Entry With Spaces.md` becomes `collection/entry-with-spaces`.
- Truncate `/index` paths to base URL: `collection/index.md` becomes `collection`
3 changes: 2 additions & 1 deletion packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
"postbuild": "astro-scripts copy \"src/**/*.astro\"",
"benchmark": "node test/benchmark/dev.bench.js && node test/benchmark/build.bench.js",
"test:unit": "mocha --exit --timeout 30000 ./test/units/**/*.test.js",
"test:unit:match": "mocha --exit --timeout 30000 ./test/units/**/*.test.js -g",
"test": "pnpm run test:unit && mocha --exit --timeout 20000 --ignore **/lit-element.test.js && mocha --timeout 20000 **/lit-element.test.js",
"test:match": "mocha --timeout 20000 -g",
"test:e2e": "playwright test",
Expand Down Expand Up @@ -137,7 +138,7 @@
"estree-walker": "^3.0.1",
"execa": "^6.1.0",
"fast-glob": "^3.2.11",
"github-slugger": "^1.4.0",
"github-slugger": "^2.0.0",
"gray-matter": "^4.0.3",
"html-entities": "^2.3.3",
"html-escaper": "^3.0.3",
Expand Down
43 changes: 13 additions & 30 deletions packages/astro/src/content/types-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,15 @@ import {
ContentConfig,
ContentObservable,
ContentPaths,
getEntryInfo,
getContentPaths,
loadContentConfig,
NoCollectionError,
} from './utils.js';

type ChokidarEvent = 'add' | 'addDir' | 'change' | 'unlink' | 'unlinkDir';
type RawContentEvent = { name: ChokidarEvent; entry: string };
type ContentEvent = { name: ChokidarEvent; entry: URL };
type EntryInfo = {
id: string;
slug: string;
collection: string;
};

export type GenerateContentTypes = {
init(): Promise<void>;
Expand Down Expand Up @@ -123,13 +120,13 @@ export async function createContentTypesGenerator({

return { shouldGenerateTypes: true };
}
const entryInfo = getEntryInfo({
entry: event.entry,
contentDir: contentPaths.contentDir,
});
// Not a valid `src/content/` entry. Silently return.
if (entryInfo instanceof Error) return { shouldGenerateTypes: false };
if (fileType === 'unknown') {
const entryInfo = getEntryInfo({
entry: event.entry,
contentDir: contentPaths.contentDir,
// Allow underscore `_` files outside collection directories
allowFilesOutsideCollection: true,
});
if (entryInfo.id.startsWith('_') && (event.name === 'add' || event.name === 'change')) {
// Silently ignore `_` files.
return { shouldGenerateTypes: false };
Expand All @@ -140,7 +137,11 @@ export async function createContentTypesGenerator({
};
}
}
if (entryInfo.collection === '.') {
const entryInfo = getEntryInfo({
entry: event.entry,
contentDir: contentPaths.contentDir,
});
if (entryInfo instanceof NoCollectionError) {
if (['info', 'warn'].includes(logLevel)) {
warn(
logging,
Expand Down Expand Up @@ -256,24 +257,6 @@ function removeEntry(contentTypes: ContentTypes, collectionKey: string, entryKey
delete contentTypes[collectionKey][entryKey];
}

export function getEntryInfo({
entry,
contentDir,
}: Pick<ContentPaths, 'contentDir'> & { entry: URL }): EntryInfo | Error {
const rawRelativePath = path.relative(fileURLToPath(contentDir), fileURLToPath(entry));
const rawCollection = path.dirname(rawRelativePath).split(path.sep).shift();
if (!rawCollection) return new Error();

const rawId = path.relative(rawCollection, rawRelativePath);
const rawSlug = rawId.replace(path.extname(rawId), '');
const res = {
id: normalizePath(rawId),
slug: normalizePath(rawSlug),
collection: normalizePath(rawCollection),
};
return res;
}

export function getEntryType(
entryPath: string,
paths: ContentPaths
Expand Down
52 changes: 49 additions & 3 deletions packages/astro/src/content/utils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import matter from 'gray-matter';
import { slug as githubSlug } from 'github-slugger';
import type fsMod from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { createServer, ErrorPayload as ViteErrorPayload, ViteDevServer } from 'vite';
import { createServer, ErrorPayload as ViteErrorPayload, normalizePath, ViteDevServer } from 'vite';
import { z } from 'zod';
import { AstroSettings } from '../@types/astro.js';
import { AstroError, AstroErrorData } from '../core/errors/index.js';
Expand Down Expand Up @@ -40,6 +42,12 @@ type Entry = {
_internal: { rawData: string; filePath: string };
};

export type EntryInfo = {
id: string;
slug: string;
collection: string;
};

export const msg = {
collectionConfigMissing: (collection: string) =>
`${collection} does not have a config. We suggest adding one for type safety!`,
Expand Down Expand Up @@ -87,11 +95,49 @@ export async function getEntryData(entry: Entry, collectionConfig: CollectionCon
return data;
}

const flattenPath = (path: (string | number)[]) => path.join('.');
export class NoCollectionError extends Error {}

export function getEntryInfo(
params: Pick<ContentPaths, 'contentDir'> & { entry: URL; allowFilesOutsideCollection?: true }
): EntryInfo;
export function getEntryInfo({
entry,
contentDir,
allowFilesOutsideCollection = false,
}: Pick<ContentPaths, 'contentDir'> & { entry: URL; allowFilesOutsideCollection?: boolean }):
| EntryInfo
| NoCollectionError {
const rawRelativePath = path.relative(fileURLToPath(contentDir), fileURLToPath(entry));
const rawCollection = path.dirname(rawRelativePath).split(path.sep).shift();
const isOutsideCollection = rawCollection === '..' || rawCollection === '.';

if (!rawCollection || (!allowFilesOutsideCollection && isOutsideCollection))
return new NoCollectionError();

const rawId = path.relative(rawCollection, rawRelativePath);
const rawIdWithoutFileExt = rawId.replace(new RegExp(path.extname(rawId) + '$'), '');
const rawSlugSegments = rawIdWithoutFileExt.split(path.sep);

const slug = rawSlugSegments
// Slugify each route segment to handle capitalization and spaces.
// Note: using `slug` instead of `new Slugger()` means no slug deduping.
.map((segment) => githubSlug(segment))
.join('/')
.replace(/\/index$/, '');

const res = {
id: normalizePath(rawId),
slug,
collection: normalizePath(rawCollection),
};
return res;
}

const flattenErrorPath = (errorPath: (string | number)[]) => errorPath.join('.');

const errorMap: z.ZodErrorMap = (error, ctx) => {
if (error.code === 'invalid_type') {
const badKeyPath = JSON.stringify(flattenPath(error.path));
const badKeyPath = JSON.stringify(flattenErrorPath(error.path));
if (error.received === 'undefined') {
return { message: `${badKeyPath} is required.` };
} else {
Expand Down
20 changes: 10 additions & 10 deletions packages/astro/src/content/vite-plugin-content-assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import {
STYLES_PLACEHOLDER,
} from './consts.js';

function isDelayedAsset(url: URL): boolean {
function isDelayedAsset(viteId: string): boolean {
const url = new URL(viteId, 'file://');
return (
url.searchParams.has(DELAYED_ASSET_FLAG) &&
contentFileExts.some((ext) => url.pathname.endsWith(ext))
Expand All @@ -30,10 +31,10 @@ export function astroDelayedAssetPlugin({ mode }: { mode: string }): Plugin {
}
},
load(id) {
const url = new URL(id, 'file://');
if (isDelayedAsset(url)) {
if (isDelayedAsset(id)) {
const basePath = id.split('?')[0];
const code = `
export { Content, getHeadings, _internal } from ${JSON.stringify(url.pathname)};
export { Content, getHeadings, _internal } from ${JSON.stringify(basePath)};
export const collectedLinks = ${JSON.stringify(LINKS_PLACEHOLDER)};
export const collectedStyles = ${JSON.stringify(STYLES_PLACEHOLDER)};
`;
Expand All @@ -42,14 +43,13 @@ export function astroDelayedAssetPlugin({ mode }: { mode: string }): Plugin {
},
async transform(code, id, options) {
if (!options?.ssr) return;
const url = new URL(id, 'file://');
if (devModuleLoader && isDelayedAsset(url)) {
const { pathname } = url;
if (!devModuleLoader.getModuleById(pathname)?.ssrModule) {
await devModuleLoader.import(pathname);
if (devModuleLoader && isDelayedAsset(id)) {
const basePath = id.split('?')[0];
if (!devModuleLoader.getModuleById(basePath)?.ssrModule) {
await devModuleLoader.import(basePath);
}
const { stylesMap, urls } = await getStylesForURL(
pathToFileURL(pathname),
pathToFileURL(basePath),
devModuleLoader,
'development'
);
Expand Down
26 changes: 13 additions & 13 deletions packages/astro/src/content/vite-plugin-content-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@ import { pathToFileURL } from 'node:url';
import type { Plugin } from 'vite';
import type { AstroSettings } from '../@types/astro.js';
import { info, LogOptions } from '../core/logger/core.js';
import { prependForwardSlash } from '../core/path.js';
import { escapeViteEnvReferences } from '../vite-plugin-utils/index.js';
import { escapeViteEnvReferences, getFileInfo } from '../vite-plugin-utils/index.js';
import { contentFileExts, CONTENT_FLAG } from './consts.js';
import {
createContentTypesGenerator,
GenerateContentTypes,
getEntryInfo,
getEntryType,
} from './types-generator.js';
import {
Expand All @@ -20,6 +18,7 @@ import {
ContentPaths,
getContentPaths,
getEntryData,
getEntryInfo,
getEntrySlug,
parseFrontmatter,
} from './utils.js';
Expand Down Expand Up @@ -109,8 +108,8 @@ export function astroContentServerPlugin({
{
name: 'astro-content-flag-plugin',
async load(id) {
const fileUrl = new URL(prependForwardSlash(id), 'file://');
if (isContentFlagImport(fileUrl)) {
const { fileId } = getFileInfo(id, settings.config);
if (isContentFlagImport(id)) {
const observable = contentConfigObserver.get();
let contentConfig: ContentConfig | undefined =
observable.status === 'loaded' ? observable.config : undefined;
Expand All @@ -128,19 +127,19 @@ export function astroContentServerPlugin({
});
});
}
const rawContents = await fs.promises.readFile(fileUrl, 'utf-8');
const rawContents = await fs.promises.readFile(fileId, 'utf-8');
const {
content: body,
data: unparsedData,
matter: rawData = '',
} = parseFrontmatter(rawContents, fileUrl.pathname);
} = parseFrontmatter(rawContents, fileId);
const entryInfo = getEntryInfo({
entry: fileUrl,
entry: pathToFileURL(fileId),
contentDir: contentPaths.contentDir,
});
if (entryInfo instanceof Error) return;

const _internal = { filePath: fileUrl.pathname, rawData };
const _internal = { filePath: fileId, rawData };
const partialEntry = { data: unparsedData, body, _internal, ...entryInfo };
const collectionConfig = contentConfig?.collections[entryInfo.collection];
const data = collectionConfig
Expand All @@ -157,7 +156,7 @@ export const slug = ${JSON.stringify(slug)};
export const body = ${JSON.stringify(body)};
export const data = ${devalue.uneval(data) /* TODO: reuse astro props serializer */};
export const _internal = {
filePath: ${JSON.stringify(fileUrl.pathname)},
filePath: ${JSON.stringify(fileId)},
rawData: ${JSON.stringify(rawData)},
};
`);
Expand All @@ -172,7 +171,7 @@ export const _internal = {
) {
// Content modules depend on config, so we need to invalidate them.
for (const modUrl of viteServer.moduleGraph.urlToModuleMap.keys()) {
if (isContentFlagImport(new URL(modUrl, 'file://'))) {
if (isContentFlagImport(modUrl)) {
const mod = await viteServer.moduleGraph.getModuleByUrl(modUrl);
if (mod) {
viteServer.moduleGraph.invalidateModule(mod);
Expand All @@ -183,7 +182,7 @@ export const _internal = {
});
},
async transform(code, id) {
if (isContentFlagImport(new URL(id, 'file://'))) {
if (isContentFlagImport(id)) {
// Escape before Rollup internal transform.
// Base on MUCH trial-and-error, inspired by MDX integration 2-step transform.
return { code: escapeViteEnvReferences(code) };
Expand All @@ -193,6 +192,7 @@ export const _internal = {
];
}

function isContentFlagImport({ searchParams, pathname }: Pick<URL, 'searchParams' | 'pathname'>) {
function isContentFlagImport(viteId: string) {
const { pathname, searchParams } = new URL(viteId, 'file://');
return searchParams.has(CONTENT_FLAG) && contentFileExts.some((ext) => pathname.endsWith(ext));
}
17 changes: 16 additions & 1 deletion packages/astro/test/content-collections.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,22 @@ describe('Content Collections', () => {
'columbia.md',
'endeavour.md',
'enterprise.md',
'promo/launch-week.mdx',
// Spaces allowed in IDs
'promo/launch week.mdx',
]);
});

it('Handles spaces in `without config` slugs', async () => {
expect(json).to.haveOwnProperty('withoutConfig');
expect(Array.isArray(json.withoutConfig)).to.equal(true);

const slugs = json.withoutConfig.map((item) => item.slug);
expect(slugs).to.deep.equal([
'columbia',
'endeavour',
'enterprise',
// "launch week.mdx" is converted to "launch-week.mdx"
'promo/launch-week',
]);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ publishedDate: 'Sat May 21 2022 00:00:00 GMT-0400 (Eastern Daylight Time)'
tags: ['announcement']
---

import './launch-week-styles.css';
import './_launch-week-styles.css';

Join us for the space blog launch!

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { getEntryInfo } from '../../../dist/content/utils.js';
import { expect } from 'chai';

describe('Content Collections - getEntryInfo', () => {
const contentDir = new URL('src/content/', import.meta.url);

it('Returns correct entry info', () => {
const entry = new URL('blog/first-post.md', contentDir);
const info = getEntryInfo({ entry, contentDir });
expect(info.id).to.equal('first-post.md');
expect(info.slug).to.equal('first-post');
expect(info.collection).to.equal('blog');
});

it('Returns correct slug when spaces used', () => {
const entry = new URL('blog/first post.mdx', contentDir);
const info = getEntryInfo({ entry, contentDir });
expect(info.slug).to.equal('first-post');
});

it('Returns correct slug when nested directories used', () => {
const entry = new URL('blog/2021/01/01/index.md', contentDir);
const info = getEntryInfo({ entry, contentDir });
expect(info.slug).to.equal('2021/01/01');
});

it('Returns correct collection when nested directories used', () => {
const entry = new URL('blog/2021/01/01/index.md', contentDir);
const info = getEntryInfo({ entry, contentDir });
expect(info.collection).to.equal('blog');
});

it('Returns error when outside collection directory', () => {
const entry = new URL('blog.md', contentDir);
expect(getEntryInfo({ entry, contentDir }) instanceof Error).to.equal(true);
});

it('Silences error on `allowFilesOutsideCollection`', () => {
const entry = new URL('blog.md', contentDir);
const entryInfo = getEntryInfo({ entry, contentDir, allowFilesOutsideCollection: true });
expect(entryInfo instanceof Error).to.equal(false);
expect(entryInfo.id).to.equal('blog.md');
});
});
Loading

0 comments on commit bf210f7

Please sign in to comment.