Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Executing loader hook functions #518

Closed
coreyfarrell opened this issue May 21, 2020 · 7 comments
Closed

Executing loader hook functions #518

coreyfarrell opened this issue May 21, 2020 · 7 comments

Comments

@coreyfarrell
Copy link
Member

istanbuljs/nyc#1320 will add rudimentary support for nyc --all including ES modules. I think to complete support I need the ability to retrieve the source which would eventually be executed for an import() of an absolute file URL without actually executing it.

I'm not sure how much of the loader hook API is needed. For example a single async function esmGetSource(specifier, context) would be enough to support what nyc needs. I'm not sure if other potential users would need the individual loader hook functions to run each step of resolve, getFormat, getSource and finally transformSource. nyc would need to know the context.url given to transformSource as well as the source code produced after transformSource ran.

@giltayar
Copy link

@coreyfarrell you can do what I did in the loader for testdouble (explained here: https://dev.to/giltayar/mock-all-you-want-supporting-es-modules-in-the-testdouble-js-mocking-library-3gh1). Since you have a loader, you can await import(the-module-you want + '__nycresolvepath') and then hook the getSource to get at the source using the defaultGetSource, and then throw an exception that will be caught in the code that awaited the import.

A better option is the upcoming import.meta.resolve which fully resolves the specifier to the file, which you can then read. Unfortunately, import.meta.resolve is still behind a flag, but you can use the first hacky option until we get import.meta.resolve with no flag.

@GeoffreyBooth
Copy link
Member

Hi, I'd like to try to get a consolidated To Do list for loaders. We currently have https://github.com/nodejs/modules/projects/8; is that up to date? If it's missing any must-haves that you think loaders need in order to unflag, can you create issues for those features (if they don't exist already) and add them to the board?

@coreyfarrell
Copy link
Member Author

@giltayar I'm not sure writing a loader hook can help nyc in this. The issue is that we need to handle when an nyc user provides their own loader hook. In the nyc --all process we use the glob module to find a list of all files (filesystem paths) which the user wants coverage. We need to find the code which node.js would execute upon require(filename) or import(pathToFileURL(filename)). Right now my proposed PR to nyc will caus
I don't think import.meta.resolve would help in this situation as we need to capture the post-transformSource code.

@GeoffreyBooth One question it looks like I can't add PR's from nodejs/node to that project? I'll make some time in the near-term to review the tickets which are listed and make sure I don't see anything missing.

@GeoffreyBooth
Copy link
Member

One question it looks like I can't add PR's from nodejs/node to that project?

Hmm. Maybe just make issues here that are nothing more than links to the PRs?

@giltayar
Copy link

giltayar commented May 24, 2020

@coreyfarrell I'm not sure I understand why not, so let me try and outline my solution in a detailed manner:

// nyc-loader.mjs
async function transformSource(source, {url}, defaultTransformSource) {
  const transformedSource = defaultTransformSource(...)
   if (url.includes('__nyc_get_transformed_Source')) {
      const error = new Error('error created with transformed source')
      error.transformedSource = transformedSource
      throw error
   } else {
       return transformedSource
   }
}

// code in nyc where you want the transformed source:
const tranformedSource = await import('module-you-want-to-get-the-source-for?__nyc_get_transformed_Source').catch(err => err.transformedSource)

@coreyfarrell
Copy link
Member Author

@giltayar nyc performs --all processing in it's main process. So when you run nyc --all=true mocha, this runs nyc which then adds hooks to child processes before executing mocha. nyc cannot add a loader hook for this purpose without spawning a new process (possible but undesirable).

The blocker for this approach is that --experimental-loader cannot be used multiple times. Lets say someone publishes a coffee-esm-loader module. I need a way to deal with the following:

cross-env NODE_OPTIONS=--experimental-loader=coffee-esm-loader nyc --all=true mocha

This hypothetical coffee-esm-loader would use transformSource to convert coffee script to JS. This converted result is what nyc needs, but cannot get in the way you suggest since there can only be a single --experimental-loader.

@giltayar
Copy link

@coreyfarrell totally understand that. But my assumption that when support for pipelining loaders will (if?) happens, then the source you get is the source up to your loader in the pipeline (so you won't get the transformations that happen after your place in the pipeline, but you will get the transformations that happen before your place in the pipeline). In this case, whomever decides the order in the pipeline gets to decide what your loader sees.

So in your example, the cofeescript loader should come before your loader, and so you get the correct source, after the coffeescript transpilation.

And if there will be no support for pipelining loaders? I believe there will be a coup d'etat if that happens 😉, but in that case you're the only one loader, and what I say still holds.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Custom Loading
Awaiting triage
Development

No branches or pull requests

3 participants