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

WIP - Docusaurus v3 + FB preset patch #1094

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

slorber
Copy link

@slorber slorber commented Oct 5, 2023

POC of Docusaurus v3 + FB preset fixes for FB sites using Algolia search instead of Lunr

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 5, 2023
Comment on lines 9 to 16
+ result.themes.push(function() {
+ return {
+ name: 'docusaurus-fb-search-theme',
+ getThemePath: () => {
+ return path_1.default.resolve(__dirname, './theme-search');
+ },
+ };
+ });
Copy link
Author

Choose a reason for hiding this comment

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

@antonk52 need to create a separate conditional theme for lunr SearchBar

Copy link
Contributor

Choose a reason for hiding this comment

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

This is really impressive. I apply these changes to the original theme and create a new version this week. Sorry about having to check out the output code.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've applied your suggested changes and released docusaurus-plugin-internaldocs-fb@1.16.0

Hope this helps

Comment on lines 33 to 36
diff --git a/node_modules/docusaurus-plugin-internaldocs-fb/theme/SearchBar/DocSearch.d.ts b/node_modules/docusaurus-plugin-internaldocs-fb/theme-search/SearchBar/DocSearch.d.ts
similarity index 100%
rename from node_modules/docusaurus-plugin-internaldocs-fb/theme/SearchBar/DocSearch.d.ts
rename to node_modules/docusaurus-plugin-internaldocs-fb/theme-search/SearchBar/DocSearch.d.ts
Copy link
Author

Choose a reason for hiding this comment

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

@antonk52 need to move theme/SearchBar to theme-search/SearchBar

@slorber slorber changed the title Docusaurus v3 + fb preset patch WIP - Docusaurus v3 + FB preset patch Oct 5, 2023
@slorber slorber marked this pull request as draft October 5, 2023 17:36
Comment on lines +1 to +4
{
"snippets": {},
"description": "@generated"
}
Copy link
Author

Choose a reason for hiding this comment

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

@antonk52 what is this file? Should it be commited?

Copy link
Contributor

Choose a reason for hiding this comment

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

This file is generated by docusaurus-plugin-internaldocs-fb, it should be checked in. It is used to store some dynamic content between the builds internally. Externally it won't change after the initial check in. Please do not add it to .gitignore file.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, but why isn't it checked in currently? 😅

Should I add it as part of this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's already checked in internally (ie, to our Sapling monorepo), but the fb directory is excluded from syncing with GitHub. It shouldn't be in the PR - just leave it unstaged or locally ignored.

(Tbh I'm not sure what would happen if I tried to import it 😅. I guess it would just be discarded.)

@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (112e8c7) 83.38% compared to head (0d3df03) 83.38%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1094   +/-   ##
=======================================
  Coverage   83.38%   83.38%           
=======================================
  Files         209      209           
  Lines       10839    10839           
  Branches     2726     2726           
=======================================
  Hits         9038     9038           
  Misses       1801     1801           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -24,14 +24,16 @@ Our recommended workflow is to use [`yarn link`][1] to register local `metro` pa

We recommend using `npm exec --workspaces` to register all packages in the `metro` repo — these can be individually linked into the target project later.

npm exec --workspaces -- yarn link
```bash
Copy link
Author

Choose a reason for hiding this comment

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

Preferable to use sh than bash to be consistent with other code blocks in this file


2. **Use `yarn link` to replace Metro packages in your target project**

From inside our target project folder, `yarn link <package-name>` can be used to apply our registered `metro` packages for that project only.

```sh
# Links 3 packages
# Links 3 packages
Copy link
Author

Choose a reason for hiding this comment

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

?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants