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

Plan for deprecation & removal of doc versioned id #6018

Closed
2 tasks done
slorber opened this issue Nov 26, 2021 · 8 comments · Fixed by #9310
Closed
2 tasks done

Plan for deprecation & removal of doc versioned id #6018

slorber opened this issue Nov 26, 2021 · 8 comments · Fixed by #9310
Labels
proposal This issue is a proposal, usually non-trivial change
Milestone

Comments

@slorber
Copy link
Collaborator

slorber commented Nov 26, 2021

Have you read the Contributing Guidelines on issues?

Motivation

As part of #5830 I had to start a refactor we wanted to do for a long time:

  • Docs versioned sidebars are not normalized anymore when created (ie category and docs shorthand are kept, default options are not "freezed")
  • Not adding a version prefix in versioned sidebar items
  • Using an unversioned docId everywhere ("dir/doc") instead of a versioned docId ("version-1.0.0/dir/doc")

The implementation in this PR is just the beginning:

  • New sidebars are copied un-normalized
  • Allow using both versioned / unversioned doc ids for retrocompatibility

We should now:

  • Deprecating the doc.id asap with a clear warning telling the user to upgrade the sidebar files (ie removing the version prefixes)
  • Plan a breaking change in a few months
  • Do the cleanup: remove doc.id, then rename doc.unversionedId to doc.id

Or maybe we could do the breaking change / cleanup immediately, considering it's not too painful for users to remove the sidebar prefix?
(but how would they discover what they need to do, we probably still need a good error message in our code?)

Self-service

  • I'd be willing to do some initial work on this proposal myself.
@slorber slorber added proposal This issue is a proposal, usually non-trivial change status: needs triage This issue has not been triaged by maintainers labels Nov 26, 2021
@Josh-Cena Josh-Cena removed the status: needs triage This issue has not been triaged by maintainers label Nov 27, 2021
@Josh-Cena
Copy link
Collaborator

Or maybe we could do the breaking change / cleanup immediately, considering it's not too painful for users to remove the sidebar prefix?
(but how would they discover what they need to do, we probably still need a good error message in our code?)

I'm for an immediate breaking change. Once we get legacy code intertwined with useful code it's very hard to clean up. Also, prompting users to upgrade with a warning is no different from prompting them with an error; up-to-date users are going to do it and casual ones will ignore it until the warning turns into an error.

The migration could be painful considering it's not a simple search & replace. Maybe we should provide another CLI command in the docs plugin to do it for the users? This CLI command can be removed in the future.

I think docusaurus upgrade should, in the future, be a general purpose CLI that automatically fixes these fixable breaking changes, but before #4082 is actually merged, let's make a makeshift command

@slorber
Copy link
Collaborator Author

slorber commented Dec 1, 2021

The migration could be painful considering it's not a simple search & replace.

Wouldn't just searching for 'version-*/' in versioned_sidebars be good enough? It does not seem very painful and worth it to build a CLI for that but maybe I'm missing something?

@Josh-Cena
Copy link
Collaborator

Wouldn't just searching for 'version-*/' in versioned_sidebars be good enough?

Haha, yeah, regex replace itself would be a bit too much, and it would probably be version-[^/]+/... But anyways, I definitely want a breaking change, let's see if people actually complain about migration difficulty

@slorber slorber added this to the 2.0.0 milestone Dec 17, 2021
@slorber slorber changed the title Plan for deprecation & removal of doc versioned id [2.0] Plan for deprecation & removal of doc versioned id Dec 17, 2021
@Josh-Cena
Copy link
Collaborator

@slorber I think we should also remove the coercion of prefixing IDs with the directory. frontMatter.id should be directly used as the doc's ID. I noticed you already left an inline comment, so just keeping it here to remind us that we should do this as well.

@slorber
Copy link
Collaborator Author

slorber commented Jan 27, 2022

Yes, agree on that, frontMatter.id should be used without prefixing by folders breadcrumb.

An explicit id should not change when you move a doc around, and you should not need to edit the sidebar when a doc changes place

This is another breaking change that will annoy users.
Do we want to do both at the same time? that could be better IMHO

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Jan 28, 2022

Let's do a breaking change for all before the official release. I wonder if many are using the explicit ID anyways.

@slorber
Copy link
Collaborator Author

slorber commented Jan 28, 2022

Checked and 3/3 so far 😅 :

But only Relay uses id + nest docs in subfolders, so not all will be affected

This is going to be quite annoying (particularly for Relay, a lot of versions to edit), so we'd rather add first a deprecation warning on something so that users can easily update all those ids.

Let's use Relay as a test site to see how painful it is to migrate

@Josh-Cena
Copy link
Collaborator

@slorber I thought about it. The only way for this breaking change to be progressive is if we allow two IDs to be equally valid for a doc (one being versioned and with the folder path appended, the other with neither), and warn about the previous.

@facebook facebook deleted a comment from Jelboy15 Mar 9, 2022
@slorber slorber modified the milestones: 2.0.0, 3.0 Aug 17, 2023
@slorber slorber changed the title [2.0] Plan for deprecation & removal of doc versioned id Plan for deprecation & removal of doc versioned id Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue is a proposal, usually non-trivial change
Projects
None yet
2 participants