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

What about Yarn PnP support? #176

Closed
koshic opened this issue Oct 17, 2022 · 13 comments
Closed

What about Yarn PnP support? #176

koshic opened this issue Oct 17, 2022 · 13 comments

Comments

@koshic
Copy link
Collaborator

koshic commented Oct 17, 2022

Any loader used in PnP mode can't resolve/load any packages except Node builtins. Yep, we have discussion in Node community how to improve loaders, how to redesign API, etc.

But now only one way - to have bundled loader. It works perfectly for me (I use simple esdbuild-based transpiler for .ts, as an example), but I can't use esmock 'out of box' (proxy bundle is the way, but...) to try node native test runner instead of Jest.

Can we remove single external dependency, 'resolvewithplus' here? Or, mark it optional and use import.meta.resolve (by flag / different import path like esmock/native or so on).

@iambumblehead
Copy link
Owner

It might be OK to embed resolvewithplus inside esmock. Maybe the prepublish script could copy resolvewithplus.js into the src folder then update esmock files to import './resolvewithplus.js' rather than 'resolvewithplus'. A new version could then be published with no dependencies.

@tripodsan @aladdin-add it would be good to know what you think of this idea.

@iambumblehead
Copy link
Owner

@koshic does resolvewithplus need to be removed from package.json? If resolvewithplus is not removed from package.json, but if esmock does something like this, would this work for your situation?

const resolver = typeof import.meta.resolve === 'function'
  ? import.meta.resolve
  : (await import('resolvewithplus')).default

@koshic
Copy link
Collaborator Author

koshic commented Oct 17, 2022

@jakebailey I think the second one is the best solution, with marking resolvewithplus as optional peer dependency, thx!

ps it should work :)

@iambumblehead
Copy link
Owner

"import.meta.resolve" does not resolve subpaths. The test creates a call like await import.meta.resolve('#sub', parent)

import.meta.resolve is experimental and, to my understanding based on browsing various discussions in nodejs upstream, incomplete. If esmock is updated to rely on current import.meta.resolve, things may get messy in the future.

@iambumblehead
Copy link
Owner

there will be less maintenance, if we wait for import.meta.resolve to be stable and complete before using it.

@iambumblehead
Copy link
Owner

Or, mark it optional and use import.meta.resolve (by flag / different import path like esmock/native or so on).

a good idea BUT esmock tried exporting something like this a month or two ago and typescript does not support this sort of thing TypeStrong/ts-node#1877 even if typescript could (or can?) do this, the support is or will not be great and annoying complexity necessary to placate typescript

@koshic
Copy link
Collaborator Author

koshic commented Oct 18, 2022

"import.meta.resolve" does not resolve subpaths. The test creates a call like await import.meta.resolve('#sub', parent)

It supports:
image

@iambumblehead
Copy link
Owner

thanks for making the demonstration. I'll try updating the PR later today and then respond afterward

@iambumblehead
Copy link
Owner

I think the problem at the PR is, "parent" is not in fileURL format that starts with "file:///" this can be fixed of course

@koshic
Copy link
Collaborator Author

koshic commented Oct 18, 2022

there will be less maintenance, if we wait for import.meta.resolve to be stable and complete before using it.

--experimantal-loaders is experimental too, so waiting can be looong )

@iambumblehead
Copy link
Owner

@koshic if you approve this PR, let's merge and use follow-up PRs to specifically resolve the two issues,

  1. Invalid check in esmockIsLoader.js #178 use loader mechanism directly to detect esmock loader,
  2. What about Yarn PnP support? #176 (this ticket) make resolvewithplus an optional dependency

I have not used optional dependencies myself so am interested to see how it works. If resolvewithplus is an optional dependency, will npm continue to download it by default for esmock? My research indicates resolvewithplus will continue to be downloaded in the default case, but I haven't used optional dependencies before so am not certain.

@koshic if you still have test files you made for testing stub export, would you test if import.meta.resolve returns paths for moduleIds with extensions like "tsx" or "json"? If you're not interested to test this that's good and I can make a little test later this evening or tomorrow.

@koshic
Copy link
Collaborator Author

koshic commented Oct 19, 2022

@iambumblehead I will approve after few small fixes )

And yes, import.meta.resolve returns all paths:
image

@iambumblehead
Copy link
Owner

changes are published and deployed https://github.com/iambumblehead/esmock/releases/tag/v2.0.7

feel free to re-open if there is any issue :)

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

No branches or pull requests

2 participants