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

feat(v2): blog slug frontmatter #3284

Merged
merged 1 commit into from
Aug 20, 2020
Merged

feat(v2): blog slug frontmatter #3284

merged 1 commit into from
Aug 20, 2020

Conversation

JeanMarcSaad
Copy link
Contributor

Motivation

Solving the slug issue raised in issue #3230

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

The code changes affected the interaction with blog post url slugs.
They now operate in the following manner:

  • If a slug option is not passed:
    • If the file name matches the pattern YYYY-MM-DD-blog-name.md, the blog will be accessible at /blog/YYYY/MM/DD/blog-name
    • Else, the slug is evaluated to be the file name. Example test-blog.md, is accessible at /blog/test-blog
  • If a slug option is passed, the slug takes that value. Example slug: 2020/08/14/test-blog will be accessible at /blog/2020/08/14/test-blog

These changes and different situations were tested manually by usingyarn test:build:v2 which deployed the packages to a local verdaccio server, and created a test-website docusaurus project where I could run different blog post slug tests.

image

image

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Aug 14, 2020
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Aug 14, 2020

Deploy preview for docusaurus-2 ready!

Built with commit 76073b8

https://deploy-preview-3284--docusaurus-2.netlify.app

Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good to me, but I'd rather not format the slug provided by the user: he's responsible to provide a valid one (like for docs).

Also, can you add more tests?
I'd like to see tests related to slugs, with different kind of slugs (with/without leading /, with/without special chars etc).

If something is easy to test, it should rather be tested. I'm not a regexp expert, intuitively I don't know what formatSlug does exactly, but tests would tell me by examples.

routeBasePath,
frontMatter.id || toUrl({date, link: linkName}),
]),
permalink: normalizeUrl([baseUrl, routeBasePath, formatSlug(slug)]),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I'd be more comfortable if you did not format the slug if it's provided by frontmatter.

The docs plugin does not do any normalization either so I prefer it to be consistent.

The user should be able to use slug: /hey/my super path/héllô and it should work

@slorber slorber changed the title feat: allow better control over blog slugs feat(v2): allow better control over blog slugs Aug 20, 2020
@slorber slorber changed the title feat(v2): allow better control over blog slugs feat(v2): blog slug frontmatter Aug 20, 2020
@slorber slorber added the pr: new feature This PR adds a new API or behavior. label Aug 20, 2020
@slorber
Copy link
Collaborator

slorber commented Aug 20, 2020

Thanks :) tested and it works fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants