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

Yarn PnP support #58

Closed
koshic opened this issue Oct 11, 2023 · 4 comments
Closed

Yarn PnP support #58

koshic opened this issue Oct 11, 2023 · 4 comments

Comments

@koshic
Copy link
Collaborator

koshic commented Oct 11, 2023

As a developer I want to use Yarn PnP in my projects, but resolvewithplus doesn't support it at all. As a result, it's impossible to use latest esmock versions.

From the API perspective, it's easy to add PnP support https://yarnpkg.com/advanced/pnpapi (really dirty example):

const resolvewith = (moduleId, parent, opts) => {
  const pnp = require('pnpapi');
  try {
    const resolved = pnp.resolveRequest(moduleId, parent);
    if (resolved !== null) {
      console.log('RESOLVED!', resolved)
      return cache[moduleId+parent] = resolved
    }
  } catch {
  }

  let resolvedpath = cache[moduleId+parent]
  if (resolvedpath) return resolvedpath

Sadly, it's hard for me to find exact point where I can switch resolution mode depends on PnP mode flag. May be it's better to use something like that:

const resolvewithClassic = ...
const resolvewithPnP = ...
const resolvewith = isPnPEnabled ? resolvewithPnP : resolvewithClassic

Also, it looks like that we can't use options (isbrowser, priority) because package.json will be processed by Yarn loader.

@iambumblehead what do you think? I can provide test repo with Yarn PnP enabled, but it may be not so easy to setup unit tests pipeline.

@iambumblehead
Copy link
Owner

iambumblehead commented Oct 11, 2023

Thanks for opening this issue. Possibly the future could include even more package managers and more module resolutions. I don't know how orogene resolves modules, but if it uses a newer resolution, esmock and resolvewithplus might need to support that.

What do you think of this idea? esmock could provide an interface for users to add any custom resolvers. For example like this,

import esmock from 'esmock'
import pnpapi from 'pnpapi'

const esmockPnP = esmock.addResolvers([ pnpapi.resolveRequest ])
const modulePnP = await esmockPnP('../src/modulePnP.js', {
  '../src/moduleIdChildPnP.js' : () => ['a', 'b'],
})

A downside to the above approach is that it would complicate the typescript types file. A simpler approach for managing the types file would look like this,

import esmock from 'esmock'
import pnpapi from 'pnpapi'

const modulePnP = await esmock('../src/modulePnP.js', {
  '../src/moduleIdChildPnP.js' : () => ['a', 'b'],
}, null, {
  resolvers: [ pnpapi.resolveRequest ]
})

And the user could add a wrapper to simplify the call,

import esmock from 'esmock'
import pnpapi from 'pnpapi'

const esmockPnP = async (parent, localmocks, globalmocks) => (
  esmock(parent, localmocks, globalmocks, { resolvers: [ pnpapi.resolveRequest ] }))

const modulePnP = await esmockPnP('../src/modulePnP.js', {
  '../src/moduleIdChildPnP.js' : () => ['a', 'b'],
})

@koshic
Copy link
Collaborator Author

koshic commented Oct 12, 2023

Thx for quick answer!

Yeah, I agree with that approach - to pass custom resolver to esmock instead of changing resolvewithplus. In the future, resolvewithplus (or esmock, doesn't matter) can include different resolution modes if you decide that many users want it, right?

What about API changes - I think your second option is totally ok, simple wrapper for non-trivial setup is not a big deal.

Also, I propose to slightly refactor resolvewithplus usage, and switch from resolvewith.iscoremodule to https://nodejs.org/dist/latest-v20.x/docs/api/module.html#moduleisbuiltinmodulename (only one concern - old Node versions compatibility), to simplify resolver contract.

What is our next steps? I can create custom resolver for PnP and test in various projects, but I don't know when I can have enough time to create PR to esmock repo with unit tests (

@iambumblehead
Copy link
Owner

Thanks for the message and agreement on all points. In the next days, I could add custom-resolver PR to esmock following this example,

const modulePnP = await esmock('../src/modulePnP.js', {
  '../src/moduleIdChildPnP.js' : () => ['a', 'b']
}, null, {
  resolvers: [ pnpapi.resolveRequest ]
})

Then additional resolvers could be added any time. Sync and Async resolvers could be supported, but initial PR might by sync-only to start.

@iambumblehead
Copy link
Owner

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