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(v1): add deletedDocs config to fix unwanted versioning fallback #2955

Merged
merged 4 commits into from
Jun 25, 2020
Merged

feat(v1): add deletedDocs config to fix unwanted versioning fallback #2955

merged 4 commits into from
Jun 25, 2020

Conversation

aldeed
Copy link
Contributor

@aldeed aldeed commented Jun 17, 2020

Motivation

Fixes an issue with v1 versioning fallback described in #2429 and #2123

This adds support for a deletedDocs option in siteConfig, which allows you to mark article IDs as being deleted beginning with a certain version. This circumvents version fallback and also removes the article from the versionless/next URL.

Example:

{
  deletedDocs: {
    "2.0.0": new Set([
      "tagging"
    ])
  }
}

This is a backwards-compatible new feature for v1, but also essentially a "fix" for this issue that can be very serious when instructions that shouldn't be followed on later versions are still accessible in those versions.

Note that v2 uses a different approach to versioning that does not have this problem in the first place.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

  1. yarn link the v1 package into your project
  2. Add deletedDocs option in siteConfig.js for some docs that are in older versions but no longer relevant or linked from anywhere. Refer to example in the updated docs.
  3. Run start or build command and verify that those URLs have no content (error/404).

@aldeed aldeed requested a review from yangshun as a code owner June 17, 2020 19:28
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jun 17, 2020
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Jun 17, 2020

Deploy preview for docusaurus-2 ready!

Built with commit a41782b

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

@slorber
Copy link
Collaborator

slorber commented Jun 18, 2020

Thanks @aldeed that looks like a nice feature to fix the existing fallback issues of D1 we have for a long time.

About using new Set(["tagging"]) in the doc, it's the only config that would use a Set, I'd prefer to just have ["tagging"] instead (even if it seems the code could work with both anyway).

That would be nice if we were able to dogfood this feature by actually removing the fallback of some D1 website docs that are deprecated. I don't have enough history on D1 to have a doc that comes to mind but will try to find one.

@slorber
Copy link
Collaborator

slorber commented Jun 18, 2020

Unfortunately, I couldn't find any doc that seems removed between first/last version of D1 website 🤪

@aldeed
Copy link
Contributor Author

aldeed commented Jun 18, 2020

@slorber I had the same idea but also didn't see any removed docs files. I can change the Set to an array. Probably will convert to a Set when loading it, though, since that loop runs a lot and .has will be faster than .includes.

@aldeed
Copy link
Contributor Author

aldeed commented Jun 24, 2020

@slorber I updated this PR to accept an Array in the config. I do convert it to a Set before the versions loop to keep things speedy. Because the Set constructor can take another Set or an array, this means that it will actually work to use either an array or a set in the config file, but I changed the documentation to show an array. So the fact that it could be a Set is just a hidden "feature".

@slorber
Copy link
Collaborator

slorber commented Jun 25, 2020

Hi,

Using set is not a problem, but I think it's not very useful to document usage of set. People are fine with array and perf are good enough, it's a micro implementation detail that does not bring much value to document imho.


I tried this on D1 website:

{ 
 deletedDocs: {
    'version-1.13.0': ['adding-blog'],
  },
}

I deleted the "next" doc in /docs and its sidebar.

So I guess it's supposed to fallback to the last found version 1.12.

Yet, all those links still work:

Is it working, or did I do something wrong?

yarn start or yarn build, the doc is still there

@aldeed
Copy link
Contributor Author

aldeed commented Jun 25, 2020

@slorber Version key must match versions.json file. Works for me if I remove version- from what you posted. I adjusted the docs a bit to make that clear.

@slorber slorber merged commit d513dec into facebook:master Jun 25, 2020
@slorber
Copy link
Collaborator

slorber commented Jun 25, 2020

Thanks, thats seems to work fine ;)

@slorber slorber added the pr: new feature This PR adds a new API or behavior. label Jun 25, 2020
@slorber slorber changed the title Add deletedDocs config to fix unwanted versioning fallback feat(v1): add deletedDocs config to fix unwanted versioning fallback Jun 25, 2020
@aldeed aldeed deleted the feat-deleted-docs branch July 3, 2020 19:06
@slorber
Copy link
Collaborator

slorber commented Aug 1, 2020

released in https://github.com/facebook/docusaurus/releases/tag/v1.14.5

please let me know if this works, as it's my first v1 release 😅

@aldeed
Copy link
Contributor Author

aldeed commented Aug 8, 2020

@slorber I successfully configured this for a project using 1.14.6 release. Thanks!

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.

4 participants