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] Prepare scripts for migrating to new structure #30386

Merged
merged 39 commits into from
Jan 5, 2022

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Dec 24, 2021

This PR update the migration and related scripts to generate pages & APIs according to the decided URLs. eg:

  • /components/buttons => /material/react-buttons
  • /components/data-grid/data-grid => /x/data-grid/react-data-grid
  • /api/button-unstyled => /base/api/mui-base/button-unstyled

This is a part of the 1st phase in #30091

Summary

  • setup redirects and hide it under feature toggle. It will be turned on at the same time that algolia has indexed the new URLs.
  • split E2E tests into 2 files (old & new) so that it is easy to clean up.

Review recommend

  • review e2e tests
  • try the migration locally
    • yarn docs:migrate:pages && yarn docs:api (you should see 5k+ of changes, the changes won't be committed in this PR)
    • yarn docs:dev
    • yarn test:e2e-website:dev (all the tests should pass)

@siriwatknp siriwatknp changed the title [docs] Prepare scripts for migration [docs] Prepare scripts for migrating to new structure Dec 24, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Dec 24, 2021

No bundle size changes

Generated by 🚫 dangerJS against de65ae5

@mbrookes mbrookes added the docs Improvements or additions to the documentation label Dec 26, 2021
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 27, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 27, 2021
@siriwatknp siriwatknp mentioned this pull request Dec 27, 2021
32 tasks
@siriwatknp siriwatknp marked this pull request as ready for review December 27, 2021 03:56
@@ -116,7 +116,7 @@ function reduceChildRoutes(context) {

if (page.children && page.children.length > 1) {
const title = pageToTitleI18n(page, t);
const topLevel = activePage ? activePage.pathname.indexOf(`${page.pathname}/`) === 0 : false;
Copy link
Member Author

Choose a reason for hiding this comment

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

/ is not necessary which makes the new URLs impossible. eg:

{
  pathname: '/material/react-'
  children: [
    { pathname: '/material/react-buttons' },
  ]
}

The logic will compare /material/react-/ with /material/react-

{ pathname: '/material/react-use-media-query', title: 'useMediaQuery' },
],
},
{
Copy link
Member

Choose a reason for hiding this comment

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

Why are the data-grid pages here?

Copy link
Member Author

Choose a reason for hiding this comment

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

We currently have data-grid in the components. I group all data-grid related to one link so that the people who are familiar with the previous layout don't confuse.

image

paths: [
path.join(__dirname, '../../../packages/mui-lab/src'),
path.join(__dirname, '../../../packages/mui-material/src'),
path.join(__dirname, '../../../packages/mui-base/src'),
Copy link
Member

Choose a reason for hiding this comment

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

Why is base here?

Copy link
Member Author

Choose a reason for hiding this comment

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

because @mui/base is a dependency on material.

Copy link
Member Author

@siriwatknp siriwatknp Jan 4, 2022

Choose a reason for hiding this comment

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

This part is related to only components (neglect utilities, hooks or constants for now)

@mnajdova
Copy link
Member

yarn docs:migrate:pages && yarn docs:api (you should see 5k+ of changes)

How can we verify that all pages are identical as before?

@siriwatknp
Copy link
Member Author

siriwatknp commented Jan 4, 2022

yarn docs:migrate:pages && yarn docs:api (you should see 5k+ of changes)

How can we verify that all pages are identical as before?

e2e tests should verify that the change is safe by running:

  • yarn docs:dev
  • yarn test:e2e-website:dev (all the tests should pass)

@hbjORbj
Copy link
Member

hbjORbj commented Jan 4, 2022

All three commands (yarn docs:migrate:pages && yarn docs:api, yarn docs:dev, and yarn test:e2e-website:dev) work as expected and the website looks great.

I think List and Table should belong in "Layout", and Box and Container in "Surfaces". Or, has this already been discussed?

@siriwatknp
Copy link
Member Author

siriwatknp commented Jan 4, 2022

All three commands (yarn docs:migrate:pages && yarn docs:api, yarn docs:dev, and yarn test:e2e-website:dev) work as expected and the website looks great.

I think List and Table should belong in "Layout", and Box and Container in "Surfaces". Or, has this already been discussed?

Good point, we haven't discussed it and I would leave that to @danilo-leal. I don't think we need to consider it in this PR and let's keep them the same as before is safest for now.

Copy link
Member

@hbjORbj hbjORbj left a comment

Choose a reason for hiding this comment

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

Yep, we don't need to discuss that in this PR. Looks good!

@siriwatknp siriwatknp merged commit d7cf74f into mui:master Jan 5, 2022
wladimirguerra pushed a commit to wladimirguerra/material-ui that referenced this pull request Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants