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

[docs-infra] Fix <title> generation #11182

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Nov 25, 2023

A regression introduced from #10795, see #10539 for the context of what's the regression.

I didn't expect it would be easy to fix. At first, I was going to encourage a revert, using #10539 as why. But while I was refining the reproduction to raise the issue to the Next.js team vercel/next.js#55817 (comment), creating a reproduction in https://github.com/oliviertassinari/next-55817, I found by chance that following the proposed upgrade path solved the issue.

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work docs Improvements or additions to the documentation regression A bug, but worse labels Nov 25, 2023
@mui-bot
Copy link

mui-bot commented Nov 25, 2023

Deploy preview: https://deploy-preview-11182--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against f8e5d7c

@@ -105,6 +105,7 @@ module.exports = withDocsInfra({
},
};
},
distDir: 'export',
// Next.js provides a `defaultPathMap` argument, we could simplify the logic.
// However, we don't in order to prevent any regression in the `findPages()` method.
exportPathMap: () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Next.js deprecating this https://nextjs.org/docs/app/api-reference/next-config-js/exportPathMap worries me. How are we supposed to have pages under /api/ once exportPathMap is gone? But I guess that now since we have product folded prefixes in the URLs, we don't need to worry about the conflict with the Next.js backend API route. We can rename api-docs -> api everywhere (as it used to be before Next.js introduced the name conflict with its API route in v9 https://nextjs.org/blog/next-9#api-routes).

Copy link
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

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

I can confirm it works (title is present in the HTML file sent by server call) but I've know idea about why

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Nov 27, 2023

I've know idea about why

@alexfauquette Yeah, strange, moving from yarn export to output: export did the trick.

We can replicate this in https://github.com/mui/material-ui.

@oliviertassinari oliviertassinari merged commit bc04761 into mui:next Nov 27, 2023
18 checks passed
@oliviertassinari oliviertassinari deleted the fix-next-regression branch November 27, 2023 13:16
@oliviertassinari oliviertassinari changed the title [docs] Fix <title> generation [docs-infra] Fix <title> generation Nov 27, 2023
alexfauquette pushed a commit to alexfauquette/mui-x that referenced this pull request Jan 26, 2024
alexfauquette added a commit that referenced this pull request Jan 26, 2024
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work docs Improvements or additions to the documentation regression A bug, but worse scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants