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

Collector issue for known issues w/ jiti #565

Closed
webpro opened this issue Mar 21, 2024 · 11 comments
Closed

Collector issue for known issues w/ jiti #565

webpro opened this issue Mar 21, 2024 · 11 comments
Labels
bug Something isn't working

Comments

@webpro
Copy link
Collaborator

webpro commented Mar 21, 2024

There's a number of issues around using jiti. First and foremost: Jiti is awesome, Knip wouldn't nearly be as good without it!

This is a collector issue for housekeeping, and I expect those to be resolved at once. issues are documented at https://knip.dev/reference/known-issues

I think most or all of them can be potentially worked around by using bun:

bunx --bun knip

This uses the Bun runtime. Bun does not transpile, just strips types, has flexible module resolution, and also uses TS path aliases (tsconfig.json#compilerOptions.paths) in dynamically loaded scripts/modules.

jiti v2 might fix the issues for the Node.js runtime.

@stephenwade
Copy link

Jiti v2 has been released!

https://github.com/unjs/jiti/releases/tag/v2.0.0

@webpro
Copy link
Collaborator Author

webpro commented Sep 27, 2024

Yay!

The jiti-v2 branch has been revived and it would be great if you could give it shot in your project(s). Install like so with your favorite package manager:

npm i -D https://pkg.pr.new/knip@e127e23

Hopefully we can remove some of the known issues! Planning to include this in a next patch or minor update. Let me know if you think upgrading to jiti v2 should be a major bump for Knip.

cc @stephenwade @vojtechsimetka @thenbe @EvgenyOrekhov @JoshuaKGoldberg @crystalfp @splincode

@webpro
Copy link
Collaborator Author

webpro commented Sep 27, 2024

Also cc @deadcoder0904 @me4502 @andriyor @Codex-

@JoshuaKGoldberg
Copy link
Contributor

Awesome, knip@e127e23 works great for me on create-typescript-app. Thanks so much for this @webpro + Jiti folks!

@Codex-
Copy link
Contributor

Codex- commented Sep 28, 2024

Tried it with knip reporter and it seems 👌

@stephenwade
Copy link

It works great on my project as well. I no longer need to use bunx --bun knip.

Planning to include this in a next patch or minor update. Let me know if you think upgrading to jiti v2 should be a major bump for Knip.

No reason for it to be a major bump if nothing breaks in the integration workflow.

@me4502
Copy link
Contributor

me4502 commented Oct 1, 2024

Hmm I'm testing it out on a large monorepo, and removing the webpack: false line from the config still fails for me when parsing a TypeScript ESM webpack config. It complains with Reason: await is only valid in async functions and the top level bodies of modules when a top-level await is in place, but the message indicates that top level await should be working.

@webpro
Copy link
Collaborator Author

webpro commented Oct 1, 2024

Hmm I'm testing it out on a large monorepo, and removing the webpack: false line from the config still fails for me when parsing a TypeScript ESM webpack config. It complains with Reason: await is only valid in async functions and the top level bodies of modules when a top-level await is in place, but the message indicates that top level await should be working.

@me4502 Thanks for trying that out. I wrongfully assumed that the default export of jiti would be/include the new default ESM handling in v2 but instead that's in jiti.import. Should be fixed now.

npm i -D https://pkg.pr.new/knip@c07dfda

@me4502
Copy link
Contributor

me4502 commented Oct 2, 2024

Thanks, that fixed the issue :)

I did encounter some oddities around some custom webpack plugins we have, where they were compiled to JS but called out to libs that were TS- but that's probably a setup that only works for builds in the first place because ts-node doesn't care, rather than something that should work. I managed to work around it by having them compile their dependencies, but just thought I'd mention it.

Otherwise this works super well, and allowed us to remove a bunch of ignored dependencies in our knip config. Thanks :)

@webpro webpro closed this as completed in 3dd8ea6 Oct 2, 2024
@webpro
Copy link
Collaborator Author

webpro commented Oct 3, 2024

🚀 This issue has been resolved in v5.31.0. See Release 5.31.0 for release notes.

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

@webpro
Copy link
Collaborator Author

webpro commented Oct 3, 2024

Thanks everyone for helping out here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants