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] Fix Next.js v13.5.1 <title> SEO regression #40302

Merged
merged 3 commits into from
Feb 9, 2024

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Dec 24, 2023

Fix the SEO regression introduced from #40423

Use the same fix as in mui/mui-x#11182.

Preview: https://deploy-preview-40302--material-ui.netlify.app/

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation dependencies Update of dependencies labels Dec 24, 2023
@mui-bot
Copy link

mui-bot commented Dec 24, 2023

Netlify deploy preview

https://deploy-preview-40302--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 1c61afe

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 25, 2023
CONTRIBUTING.md Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 9, 2024
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work regression A bug, but worse and removed dependencies Update of dependencies labels Feb 9, 2024
@oliviertassinari oliviertassinari changed the title [docs] Update Next.js to v13.5.6 [docs] Fix Next.js v13.5.1 <title> SEO regression Feb 9, 2024
Co-authored-by: Sam Sycamore <71297412+samuelsycamore@users.noreply.github.com>
Signed-off-by: Olivier Tassinari <olivier.tassinari@gmail.com>
@oliviertassinari
Copy link
Member Author

Rebased on HEAD

CONTRIBUTING.md Outdated Show resolved Hide resolved
Signed-off-by: Olivier Tassinari <olivier.tassinari@gmail.com>
@oliviertassinari oliviertassinari added the scope: docs-infra Specific to the docs-infra product label Feb 9, 2024
@oliviertassinari
Copy link
Member Author

Problem looks solved:

Screenshot 2024-02-09 at 16 03 53

@michaldudak michaldudak merged commit 5ae79e6 into mui:master Feb 9, 2024
21 of 22 checks passed
@oliviertassinari oliviertassinari deleted the upgrade-next-js branch February 9, 2024 19:30
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Feb 15, 2024

It looks like Netlify builds are failing 40% of the time on HEAD since we merged this PR. https://app.netlify.com/sites/material-ui/deploys?filter=master labels the origin pretty clearly to be here.

I think we need to either revert #40302 and #40423 or fix it.

Is this what #41111 tries to do? Unfortunately, it's not helping. Strange as this doesn't happen on MUI X.

@michaldudak
Copy link
Member

michaldudak commented Feb 15, 2024

@Janpot found a failing build prior to this change: https://app.netlify.com/sites/material-ui/deploys/65c3bb5ccb8fee00087cbe74

We're trying different approaches to handle this and you can see our attempts among PRs.

@Janpot
Copy link
Member

Janpot commented Feb 15, 2024

@Janpot found a failing build prior to this change: https://app.netlify.com/sites/material-ui/deploys/65c3bb5ccb8fee00087cbe74

And this is what led me to believe it could've been connected to the next.config.mjs change that was merged earlier that day. Sadly reverting back to next.config.js did produce a failing build as well.

The change right before that is the Next.js update. We could indeed try reverting to 13.4.19.

update: nope, reverting Next.js produced a failing build

Otherwise I guess we could start bissecting and open a PR that reverts to a specific commit, and go backwards until we can't reproduce anymore. Or until we can safely state it's a platform issue on latest netlify images.

04b3bf8b4527b843f2f2bf52278f763d6f80ec41: ✅
3cd55d3c9245348940e4235713eb50c8d09d4a02: ✅
bcb6ef282642490942d5ffab3e324c095c06e82f: ✅

I'm stopping this effort unless #41132 turns out not to fix it

"build:clean": "rimraf .next && pnpm build",
"build-sw": "node ./scripts/buildServiceWorker.js",
"dev": "next dev",
"deploy": "git push -f material-ui-docs master:latest",
"export": "rimraf docs/export && next export --threads=3 -o export && pnpm build-sw",
Copy link
Member

@Janpot Janpot Feb 16, 2024

Choose a reason for hiding this comment

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

@oliviertassinari Any idea what caused us to add this --threads=3? It doesn't seem to translate anywhere in the output: 'export' setup.

edit: got it, netlify resource issues it seems

edit 2: checked the next.js source code and it seems that option is now coming from experimental.cpus.

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.

5 participants