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: add jsx extensions for next default file convention #578

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

Jaxenormus
Copy link
Contributor

@Jaxenormus Jaxenormus commented Apr 1, 2024

This is currently flagged as a false positive since any default.tsx or default.jsx file is not included in the entry point of the project but are also not imported since this is a special next.js file which marks it as unused by knip.

@Jaxenormus Jaxenormus marked this pull request as ready for review April 1, 2024 05:16
@webpro
Copy link
Collaborator

webpro commented Apr 1, 2024

Thanks for the PR! I'll look into it tomorrow, but iirc there was a similar discussion recently where the file turned out to be used by some plugin (not necessarily next/Next.js itself). Any chance you could confirm and/or point me to docs that do?

@Jaxenormus
Copy link
Contributor Author

Jaxenormus commented Apr 2, 2024

Thanks for the PR! I'll look into it tomorrow, but iirc there was a similar discussion recently where the file turned out to be used by some plugin (not necessarily next/Next.js itself). Any chance you could confirm and/or point me to docs that do?

I assume your referring to this discussion only one i could find that was related to this on the repo, next.js doesn't explicitly state in the docs that the extension could also be jsx or tsx but in one of their usage examples for modals they have you write a default.tsx file and in our case this is exactly what we are doing and running into the false positive

CleanShot 2024-04-02 at 15 19 33@2x

@webpro
Copy link
Collaborator

webpro commented Apr 3, 2024

Thanks again for the PR and all the details, let's merge this.

@webpro webpro merged commit bd12fe1 into webpro-nl:main Apr 3, 2024
8 of 11 checks passed
@webpro
Copy link
Collaborator

webpro commented Apr 3, 2024

🚀 This pull request is included in v5.7.1. See Release 5.7.1 for release notes.

webpro pushed a commit that referenced this pull request Apr 3, 2024
* Update index.ts

* Update file count in Next.js plugin test
@VladSez
Copy link

VladSez commented Apr 4, 2024

Sorry, not sure 100% if this is related but after updating to [email protected] next route handler is flagged now as unused export.
I am using knip --include-entry-exports

On [email protected] this is not reported and works as expected

image

@webpro
Copy link
Collaborator

webpro commented Apr 4, 2024

How about v5.7.3? 😜

@VladSez
Copy link

VladSez commented Apr 4, 2024

It works! Thank you :)

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