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: automate migration from v1 to v2 #3015

Merged
merged 76 commits into from
Jul 24, 2020

Conversation

anshulrgoyal
Copy link
Contributor

@anshulrgoyal anshulrgoyal commented Jun 30, 2020

Motivation

Try to migrate most of the website components automatically. Migrating pages has limited support.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

By adding tests and attempting on different websites using D1. I tried the migration script on jest, relay, express-validator,formik,scalafmt and webdriverio

Related PR

#3026 contains documentation

Spreadsheet

Summary of migration attempts on many sites:

https://docs.google.com/spreadsheets/d/1XJQ8stt_hm3Xa1iqyAxy71ZyLyEJxIrxLCdqBZOikh8/edit#gid=0

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

Deploy preview for docusaurus-2 ready!

Built with commit ff69e5c

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

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Jun 30, 2020

Deploy preview for docusaurus-2 ready!

Built with commit c42af01

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

@anshulrgoyal anshulrgoyal changed the title Anshul/migration feat: automate migration from v1 to v2 Jun 30, 2020
@anshulrgoyal
Copy link
Contributor Author

anshulrgoyal commented Jun 30, 2020

This was produced without any manual work
image
image

if primary color exists in D1 config
image

@anshulrgoyal anshulrgoyal marked this pull request as ready for review June 30, 2020 16:24
@slorber slorber self-requested a review July 1, 2020 13:22
@slorber slorber added the pr: new feature This PR adds a new API or behavior. label Jul 1, 2020
@slorber
Copy link
Collaborator

slorber commented Jul 1, 2020

Hey, that's cool, will try to check that in details soon.

If you can split a bit the main process in multiple smaller functions that would help to make the migration workflow more readable.

I think it's better to not automate the move of docs to the website folder, but keep it outside for now (like v1) with the correct plugin option.

Don't hesitate to create site fixtures for tests. Before a test you would copy a whole D1 sample site to a temp folder, and run the migration here and see if D2 site can build after migration. D1 sites on which the migration cli fails currently would be a good edge test case.

Also, it could be nice if we setup e2e tests to make so that any newly initialized D1 website could always be upgraded automatically to D2. Not very familiar with github actions but you can probably test this workflow locally by using their tooling + modifying docusaurus/.github/workflows/e2e-test.yml

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 great. You know more about md parsing and jcodeshift than me 😅

There are probably things to improve for pages and mdx but I think we can safely merge and release this. Will open another issue for improvement ideas I have.

I'd like a few minor changes before merging this, that are not related to pages and mdx. Those things are annoying details that we can see while running yarn docusaurus-migrate migrate ./website-1.x ./test-migrated

/docs folder copied

We should migrate the docs in place, preserve their history, and allow user to inspect their diff to see where the migration tool might fail (he might need to fix migration errors later if he don't notice them at first).

It's not a big deal if /docs folder stays outside of the docusaurus project, user will be able to move it easily inside if needed, using "git mv" to preserve his history if he cares about it.

For the rest of the site I think it's less important to preserve the history anyway.

Note: I only talk about the docs folder, not the versioned docs, for which it's less important to me to preserve the history as it's copied on each release anyway (and it would be difficult to achieve)

customFields

the theme-related attributes all end up in this part of the config, they should rather be filtered

update branch

Can't merge for now :)

.github/workflows/migration-cli-e2e-test.yml Show resolved Hide resolved
packages/docusaurus-migrate/src/index.ts Show resolved Hide resolved
'projectName',
'scripts',
'stylesheets',
'favicon',
Copy link
Collaborator

Choose a reason for hiding this comment

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

migrating v1 site leads to this. We should filter much more fields here, like the known theme attributes

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have added all the available theme attributes even twitter link is added to the footer

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, the v1 theme attributes have been put in the appropriate v2 theme config.

But these known v1 theme attributes shouldn't end up too in the v2 config.customFields, as they are now duplicated in the final config.

The final v2 config should barely have more than 2 or 3 custom fields.

Look at D2 website, it only has one, yet it was migrated a few years ago from v1 site, so migrating automatically v1 site with the cli should rather give us a quite similar output.

image

Copy link
Contributor

@teikjun teikjun Jul 23, 2020

Choose a reason for hiding this comment

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

I think the remaining fields that could be useful are:

  1. translationRecruitingLink
  2. gaTrackingId
  3. facebookAppId
  4. twitterUsername
  5. highlight.theme
  6. scrollToTopOptions

The other fields are either already included in config or deprecated
(edit: just saw that it should have 2-3 custom fields, I'll just include translationRecruitingLink, gaTrackingid, facebookAppId)

packages/docusaurus-migrate/src/index.ts Outdated Show resolved Hide resolved
packages/docusaurus-migrate/src/index.ts Outdated Show resolved Hide resolved
@slorber slorber merged commit a78f703 into facebook:master Jul 24, 2020
@slorber slorber mentioned this pull request Aug 19, 2020
@slorber slorber mentioned this pull request Nov 9, 2020
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