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(content-docs): export versioning utils #6477

Merged
merged 5 commits into from
Feb 2, 2022

Conversation

milesj
Copy link
Contributor

@milesj milesj commented Jan 27, 2022

cc: @slorber

Motivation

In docusaurus-plugin-typedoc-api we support versioning by piggy-backing off the docs implementation. We did this by importing some code from lib/versions.js and duplicating code that wasn't exported, you can view that here: https://github.com/milesj/docusaurus-plugin-typedoc-api/blob/master/packages/plugin/src/plugin/version.ts

With the addition of exports in beta.15, we're no longer able to import this utilities, which puts a lot of burden on us.

[ERROR] Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './lib/versions' is not defined by "exports" in /Users/milesj/Projects/docusaurus-plugin-typedoc-api/node_modules/@docusaurus/plugin-content-docs/package.json

To remedy this, I'm doing the following:

  • Add an exports for /versions
  • Add export to some util functions
  • Re-export constants from the index (we're using them also)

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

N/A

Related PRs

N/A

@netlify
Copy link

netlify bot commented Jan 27, 2022

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: e111ef4

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

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

@github-actions
Copy link

github-actions bot commented Jan 27, 2022

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟢 Performance 93
🟢 Accessibility 100
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 92

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

@Josh-Cena Josh-Cena changed the title fix: Export versioning utils from @docusaurus/plugin-content-docs fix(content-docs): export versioning utils Jan 27, 2022
@Josh-Cena Josh-Cena added the pr: bug fix This PR fixes a bug in a past release. label Jan 27, 2022
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 seems like a reasonable thing to do.

Not sure we'll want to document this officially but we can at least restore the useful exports


I don't know all the internals of your plugin, was wondering if in the future we couldn't find a way to make it work by extending the docs plugin? maybe having a way to somehow create "virtual mdx files" or something?

Maybe you'll find this interesting: #4138 (comment)

@@ -5,6 +5,7 @@
"main": "lib/index.js",
"exports": {
"./client": "./lib/client/index.js",
"./versions": "./lib/versions.js",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"./versions": "./lib/versions.js",
"./node": "./lib/node/index.js",

I'd rather have a unique entry point for all the exposed Node.js apis.

This way it's more explicit what the API surface is and we are less likely to rename a function by mistake and break your plugin.

Does it make sense and work for you?

@milesj
Copy link
Contributor Author

milesj commented Jan 28, 2022

@slorber Change works for me. Updated.

As for the plugin, it does not use Markdown. It renders pages using React components and a JSON dataset: https://github.com/milesj/docusaurus-plugin-typedoc-api/tree/master/packages/plugin/src/components

We also piggyback off of docs components when we can. Like this is a somewhat clone of DocItem. https://github.com/milesj/docusaurus-plugin-typedoc-api/blob/master/packages/plugin/src/components/ApiItem.tsx


// APIs available to Node.js
export * from '../constants';
export * from '../versions';
Copy link
Collaborator

@slorber slorber Jan 28, 2022

Choose a reason for hiding this comment

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

for constants I'm fine with * but I'd rather have named exports for everything else, so that we don't expose publicly things by mistake

(for client, all global data hooks are exposed on purpose)

@milesj
Copy link
Contributor Author

milesj commented Jan 29, 2022

@slorber @Josh-Cena Updated.

@milesj
Copy link
Contributor Author

milesj commented Feb 1, 2022

@slorber @Josh-Cena Bumping this because of the weekend.

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.

LGTM thanks 👍

@slorber slorber merged commit 65ba551 into facebook:main Feb 2, 2022
@milesj milesj deleted the exports-versioning branch February 2, 2022 18:43
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.

4 participants