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: adjust path on tryRequire to handle dep check #674

Merged
merged 2 commits into from
Feb 7, 2024

Conversation

mmason2-godaddy
Copy link
Contributor

@mmason2-godaddy mmason2-godaddy commented Feb 7, 2024

Summary

  • Why: The checkDevDependencies function wasn't behaving properly due to the path used in the tryRequire. A require on @docusaurus/core will always return null. Opt for a tryRequire on the package.json of that file.
  • What: Adjust path on the tryRequire

Changelog

  • adjust path on tryRequire to handle dep check

Test Plan

  • Update test case with the new path

@mmason2-godaddy mmason2-godaddy requested a review from a team as a code owner February 7, 2024 17:09
Copy link
Contributor

@jpage-godaddy jpage-godaddy left a comment

Choose a reason for hiding this comment

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

Seems fine as a workaround since there's not an alternative. My main worry is that if this package changes its internal structure this could break, but if this is how Docusaurus code imports from @docusaurus/core I guess any changes to the structure would be a breaking change anyway. Another option may be to import @docusaurus/core/package.json (they could break that in the future as well, though, if they messed with the exports declarations)

@mmason2-godaddy
Copy link
Contributor Author

Seems fine as a workaround since there's not an alternative. My main worry is that if this package changes its internal structure this could break, but if this is how Docusaurus code imports from @docusaurus/core I guess any changes to the structure would be a breaking change anyway. Another option may be to import @docusaurus/core/package.json (they could break that in the future as well, though, if they messed with the exports declarations)

I'll give this a shot real quick. That's a good alternative.

@mmason2-godaddy mmason2-godaddy merged commit cee9cee into main Feb 7, 2024
7 checks passed
@mmason2-godaddy mmason2-godaddy deleted the fix-docusaurus branch February 7, 2024 18:41
agerard-godaddy pushed a commit that referenced this pull request Feb 15, 2024
* fix: adjust path on tryRequire to handle dep check

* opt for package.json path in tryRequire

(cherry picked from commit cee9cee)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants