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

fix(v2): allow relative sidebar path resolution in docs:version command #4861

Merged
merged 2 commits into from
Jun 2, 2021

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented May 28, 2021

Motivation

Resolves #4844

Because we do not seem to have a strict requirement that the sidebar file path be absolute, currently users who use relative path in the sidebarPath field will get an error when creating new docs version.

Although after merging of #4775 docs:versions command will no longer fails, and just silently create empty sidebar file. However, this is not what the user expects - copy of current sidebar file in new docs folder. So I think instead of forcing users to specify only absolute path to the sidebar file, we can automatically resolve relative path to it when cutting new docs version.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Not sure if tests are needed here?

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@lex111 lex111 added the pr: bug fix This PR fixes a bug in a past release. label May 28, 2021
@lex111 lex111 requested a review from slorber as a code owner May 28, 2021 09:50
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label May 28, 2021
@netlify
Copy link

netlify bot commented May 28, 2021

✔️ [V2]

🔨 Explore the source changes: 76a671f

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/60b7a74cf8c4cf0008cce837

😎 Browse the preview: https://deploy-preview-4861--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented May 28, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 61
🟢 Accessibility 97
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-4861--docusaurus-2.netlify.app/

@netlify
Copy link

netlify bot commented May 28, 2021

✔️ [V1]

🔨 Explore the source changes: 76a671f

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-1/deploys/60b7a74cc61dc600079a32ba

😎 Browse the preview: https://deploy-preview-4861--docusaurus-1.netlify.app

@github-actions
Copy link

github-actions bot commented May 28, 2021

Size Change: +1.25 kB (0%)

Total Size: 621 kB

Filename Size Change
website/build/assets/css/styles.********.css 88.3 kB +164 B (0%)
website/build/assets/js/main.********.js 443 kB +683 B (0%)
website/build/blog/2017/12/14/introducing-docusaurus/index.html 62.3 kB +201 B (0%)
website/build/index.html 27.1 kB +201 B (+1%)
ℹ️ View Unchanged
Filename Size Change
website/build/docs/introduction/index.html 235 B 0 B

compressed-size-action

@slorber
Copy link
Collaborator

slorber commented Jun 2, 2021

LGTM thanks 👍

Just extracted resolveSidebarPathOption so that the 2 places share the exact same path resolution logic

Also dogfooding on our site: docs use relative path, community use absolute path, so we'll notice more easily potential regresssions

@slorber slorber merged commit 35bdde3 into master Jun 2, 2021
@sriramgroot
Copy link

I too facing this issue while running npm run docusaurus docs:version 1.1.0
Iam currently using the docusaurus version as mentioned below

"@docusaurus/core": "2.0.0-alpha.72",
"@docusaurus/preset-classic": "2.0.0-alpha.72",

Under which version of docusaurus this issue is solved ?

@slorber
Copy link
Collaborator

slorber commented Jun 10, 2021

@sriramgroot will be rerleased soon but you can try the @canary npm dist tag

@sriramgroot
Copy link

sriramgroot commented Jun 10, 2021

@slorber whether this needs to be executed in terminal were the package.json is located ?

@sriramgroot
Copy link

sriramgroot commented Jun 10, 2021

@slorber

Version generation worked with this config changes sidebarPath: require.resolve('./sidebars.json') under docusaurus.config.js

@slorber
Copy link
Collaborator

slorber commented Jun 11, 2021

yes because require.resolve converts to absolute path

Or you can keep it as it used to be and use a canary: https://www.npmjs.com/package/@docusaurus/core
For example: 2.0.0-beta.aff848e87

@slorber slorber deleted the lex111/iss4844 branch August 17, 2021 17:49
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: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs:version results in Cannot find module './sidebars.json'
4 participants