-
Notifications
You must be signed in to change notification settings - Fork 600
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
base: main
Are you sure you want to change the base?
Conversation
+ result.themes.push(function() { | ||
+ return { | ||
+ name: 'docusaurus-fb-search-theme', | ||
+ getThemePath: () => { | ||
+ return path_1.default.resolve(__dirname, './theme-search'); | ||
+ }, | ||
+ }; | ||
+ }); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 [email protected]
Hope this helps
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 |
There was a problem hiding this comment.
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
{ | ||
"snippets": {}, | ||
"description": "@generated" | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 ReportAll modified lines are covered by tests ✅
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. |
docs/LocalDevelopment.md
Outdated
@@ -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 |
There was a problem hiding this comment.
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
docs/LocalDevelopment.md
Outdated
|
||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
POC of Docusaurus v3 + FB preset fixes for FB sites using Algolia search instead of Lunr