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

Invalid check in esmockIsLoader.js #178

Closed
koshic opened this issue Oct 18, 2022 · 8 comments
Closed

Invalid check in esmockIsLoader.js #178

koshic opened this issue Oct 18, 2022 · 8 comments

Comments

@koshic
Copy link
Collaborator

koshic commented Oct 18, 2022

const isloaderRe = /--(experimental-)?loader[=\s,"']*esmock/

What if esmock loaded indirectly, as part of other loader?

image

Error: process must be started with --loader=esmock

I suggest to set flag in global object inside resolve / load, or something like that.

@iambumblehead
Copy link
Owner

Loader's will use separate thread wout access to that global object nodejs/node#44710

Would something like this work for indirect loading? What do you think?

import esmock from 'esmock'

esmock.isLoaderRequired = false

esmock(...args)

@iambumblehead
Copy link
Owner

another way might be to support a call like this, where the options param defines "isLoaderRequired" as false,

esmock('./module.js', defs, globaldefs, {
  isLoaderRequired: false
})

@koshic
Copy link
Collaborator Author

koshic commented Oct 18, 2022

What is the reason to check? For my understanding, it was created to avoid setup mistakes (for users) and to make predictable environment (for author).

So, this check in not about particular Node parameters / esmock module in general, it's about esmock resolve/load hooks (both!) are loaded by Node engine.

How to achieve that? I see 2 obvious options:

  1. Composite flag like 'isLoadHookInstalled/ usResolveHookInstalled', each part can be set at the first call on the hooks. It can't provide 100% guarantee due to prev load in chain can serve all requests by its own and never call 'nextLoad / nextResolve'.
  2. import 'esmock.ebabled' in runtime- which is require special handling in resolve (ok, it's a module) and load (return {source: 'true'})

Personally, I prefer option 2 because it guarantee that we have 2 hooks and they know about esmock. Can it be a fake? Sure. Can it happen by mistake? No.

@iambumblehead
Copy link
Owner

importing and using "load" and "resolve" hooks will not result in the error message. esmock throws the error here, when esmock is called as a function only https://github.com/iambumblehead/esmock/blob/master/src/esmock.js#L6-L7

if (!esmockIsLoader())
  throw new Error('process must be started with --loader=esmock')

maybe I am misunderstanding your message, but easiest for esmock is to disable the message when called this way

esmock('./module.js', defs, globaldefs, { isLoaderDetection: false })

and inside esmock

if (opts.isLoaderDetection !== false && !esmockIsLoader())
  throw new Error('process must be started with --loader=esmock')

@koshic
Copy link
Collaborator Author

koshic commented Oct 18, 2022

I mean that esmockIsLoader() implementation is a bit naive, and relates on way how to loader added to Node loaders chain. And my proposal is to use direct interaction with 'black box' (Node + any number of loaders), by use the following code in esmockIsLoader:

try {
  const xxx = await import('any-non-existent-specifier, like flags.esmock ot enabled.esmock')
  return true;
} catch {
  return false;
}

If esmock hooks were loaded, they will handle this request (yes, few lines of code are required) and return something to 'xxx' variable (just for example).
If there is no esmock hooks, import() will fail.

100% guaranteed, right?

@iambumblehead
Copy link
Owner

@koshic an interesting and clever idea :) yes I like that

@koshic
Copy link
Collaborator Author

koshic commented Oct 18, 2022

@koshic an interesting and clever idea :) yes I like that

Yep, it's an unblockable pipe between loaders & 'user space'.

@iambumblehead
Copy link
Owner

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

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