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

feat: update mdx detection dependency list and update custom compiler… #875

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

CHC383
Copy link
Contributor

@CHC383 CHC383 commented Dec 9, 2024

close: #494

  • Expand MDX dependency detection list
  • Update compilers doc to provide instructions on manually enabling built-in MDX compiler

Copy link

pkg-pr-new bot commented Dec 10, 2024

Open in Stackblitz

npm i https://pkg.pr.new/knip@875

commit: 71df0cb

@webpro
Copy link
Collaborator

webpro commented Dec 10, 2024

Thanks for the PR, Henry! The updated condition looks fine to me, the docs I'd like to consider at some other point (they'd be out-of-date when merged already). Could you please strip the PR from the doc changes? Happy to merge then.

@CHC383 CHC383 force-pushed the feat/improve-mdx-detection branch from 84bdc10 to 71df0cb Compare December 10, 2024 17:33
@CHC383
Copy link
Contributor Author

CHC383 commented Dec 10, 2024

Thanks Lars! I have updated the PR without the doc changes.

Just curious, what do you mean by the docs are out-of-date when merged? Do you mean remark-mdx is not included in the wording? The source code link is pointing to the main branch so users could always find the latest dependency list unless the file is moved, and the key point is to provide instructions on manually enabling the built-in MDX compiler when the users' dependency doesn't fall into the hardcoded list, such as the Storybook in #494

@webpro
Copy link
Collaborator

webpro commented Dec 10, 2024

Thanks Lars! I have updated the PR without the doc changes.

Thank you!

Just curious, what do you mean by the docs are out-of-date when merged? Do you mean remark-mdx is not included in the wording? The source code link is pointing to the main branch so users could always find the latest dependency list unless the file is moved, and the key point is to provide instructions on manually enabling the built-in MDX compiler when the users' dependency doesn't fall into the hardcoded list, such as the Storybook in #494

Was thinking about automating the doc generation just like for plugins (so folks don't need to look at source code). Also wasn't fond of both code examples, since one uses a hack to import a Knip internal (unstable) and the was pseudo code from the other issue. Generally it's also nice to have single-purpose PRs.

@webpro webpro merged commit 055a2e3 into webpro-nl:main Dec 10, 2024
23 checks passed
@webpro
Copy link
Collaborator

webpro commented Dec 10, 2024

🚀 This pull request is included in v5.39.3. See Release 5.39.3 for release notes.

Using Knip in a commercial project? Please consider becoming a sponsor.

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.

Suggestion: support finding references in MDX
2 participants