-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Lexical][UI Gallery] Fix meta lexical website intern build errors #19
base: main
Are you sure you want to change the base?
[Lexical][UI Gallery] Fix meta lexical website intern build errors #19
Conversation
…ages and apis related autogenerated docs
Clone of the PR facebook/lexical#6300 |
My review is in progress 📖 - I will have feedback for you in a few minutes! |
WalkthroughThe recent changes enhance the Docusaurus configuration to support flexible documentation setups based on the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration 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.
I have reviewed your code and found 1 potential issue.
const {packagesManager} = process.env.FB_INTERNAL | ||
? {} | ||
: require('../../scripts/shared/packagesManager'); |
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.
The current implementation uses a ternary operator to conditionally import the packagesManager based on the FB_INTERNAL environment variable. While this approach works, it might lead to potential issues if the FB_INTERNAL environment variable is not set correctly or if there are differences in the package structure between internal and external builds. Consider using a more robust method for handling environment-specific imports, such as dynamic imports or separate build configurations for internal and external environments. This will ensure that the correct packages are always imported regardless of the build environment.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
/review |
PR Reviewer Guide 🔍(Review updated until commit 1fcc34e)
|
Persistent review updated to latest commit 1fcc34e |
Persistent review updated to latest commit 1fcc34e |
@coderabbitai full review |
Actions performedFull review triggered. |
PR Code Suggestions ✨
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/lexical-website/docusaurus.config.js (6 hunks)
- packages/lexical-website/sidebars.js (1 hunks)
Additional comments not posted (4)
packages/lexical-website/sidebars.js (1)
56-70
: Verify the environment variable usage.Ensure that the
FB_INTERNAL
environment variable is set correctly in all environments to avoid misconfigurations in the sidebar.Run the following script to check where
FB_INTERNAL
is set or used in the codebase:packages/lexical-website/docusaurus.config.js (3)
Line range hint
243-272
: LGTM! Conditional plugin inclusion is handled well.The logic for conditionally including the
package-docs
plugin and filtering null values is correct.
390-401
: Verify the availability of the external API link.Ensure that the external API link (
https://lexical.dev/docs/api/
) is always available and correct whenFB_INTERNAL
is true.Run the following script to check the availability of the external API link:
Verification successful
External API link is available and functioning correctly.
The external API link (
https://lexical.dev/docs/api/
) is accessible and returns a successful HTTP 200 status code when checked. This confirms that the link is correctly configured for theFB_INTERNAL
condition.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the availability of the external API link. # Test: Check the HTTP status of the external API link. Expect: HTTP 200 OK. curl -o /dev/null -s -w "%{http_code}\n" https://lexical.dev/docs/api/Length of output: 76
175-188
: Verify the functionality ofpackagesManager
.Ensure that
packagesManager
provides the necessary methods and data whenFB_INTERNAL
is false to correctly populateentryPoints
.Run the following script to check the implementation of
packagesManager
:Verification successful
packagesManager
is correctly implemented and provides necessary methods.The
packagesManager
is defined inscripts/shared/packagesManager.js
and is used extensively across the codebase. It provides methods likegetPublicPackages
, which are essential for the logic indocusaurus.config.js
. No issues found with its implementation or usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of packagesManager. # Test: Search for the implementation of packagesManager. Expect: Correct methods and data structure. ast-grep --lang javascript --pattern $'const packagesManager = $_'Length of output: 67
Script:
#!/bin/bash # Description: Search for any mention of `packagesManager` across the codebase to understand its implementation or import. # Test: Search for `packagesManager` definition or import. rg 'packagesManager' -A 5Length of output: 19229
Zero comments from coderabbit |
User description
WHAT
Fix meta lexical website intern build errors
WHY
TEST
tested locally, everything works as is for Extern Lexical Website
Description by Korbit AI
Note
This feature is in early access. You can enable or disable it in the Korbit Console.
What change is being made?
Fix meta lexical website internal build errors by conditionally requiring
packagesManager
and adjusting configurations based on theFB_INTERNAL
environment variable.Why are these changes being made?
The internal build process for the meta lexical website was failing due to the unconditional requirement of
packagesManager
. By making the inclusion ofpackagesManager
and related configurations conditional, we ensure compatibility with both internal and external build environments.PR Type
enhancement, bug fix
Description
packagesManager
and adjusting configurations based on theFB_INTERNAL
environment variable.docusaurus.config.js
to handle different configurations for internal and external builds, including entry points and plugins.sidebars.js
to link to external documentation when building internally.Changes walkthrough 📝
docusaurus.config.js
Conditional configuration for internal and external builds
packages/lexical-website/docusaurus.config.js
packagesManager
conditional based onFB_INTERNAL
environmentvariable.
entryPoints
and plugins configuration for internal builds.sidebars.js
Conditional sidebar configuration for internal builds
packages/lexical-website/sidebars.js
FB_INTERNAL
.Summary by CodeRabbit
New Features
Improvements
FB_INTERNAL
environment variable, ensuring appropriate documentation access.