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

Have presentation on loaders. #135

Closed
bmeck opened this issue Jun 19, 2018 · 13 comments
Closed

Have presentation on loaders. #135

bmeck opened this issue Jun 19, 2018 · 13 comments

Comments

@bmeck
Copy link
Member

bmeck commented Jun 19, 2018

We have a fair number of features being related to loader hooks. I'd like to add an agenda item to either an upcoming meeting or to schedule a time to present about what loaders are/can do. Right now they seem to lack concrete definition or explanation and it is possible that this will lead to confusion as we move forward.

I've prepared some slides explaining the terms to the best of my ability: a small review of requirements, review of current loader hooks, and some thoughts on a part to refactor them based upon some research I am currently doing. I will be continuing research and nothing should be seen as a hard path forward.

Feel free to comment on the slide deck I have as we are mostly trying to reach some sort of consensus and am happy to rephrase or add content as desirable. I think the important thing in this is to discuss the what of Loaders with relation to ESM so that we get a clear distinction of what is and is not a Loader. We pass around the phrase "it just works" but we should go into depth about how it works and should be careful to not allow that phrase to be a means to ignore details as we move towards and beyond feature discussions.

@bmeck bmeck added the modules-agenda To be discussed in a meeting label Jun 19, 2018
@xtuc
Copy link
Contributor

xtuc commented Jun 20, 2018

Btw if you're looking for example, I made a WASM loader here: https://github.com/wasm-tool/node-loader

Note that wasm instantiation can block some time due to executin the start function or memory initialization.

@viktor-ku
Copy link

viktor-ku commented Jun 20, 2018

@bmeck again I disagree with this approach. I believe it will create a lot of headache in the future since we are about to call all loaders for every import instead of calling the loader only if it's necessary

  • It will make loaders code more clean and loader-specific, because loader will handle one specific import type
  • Loader won't need to have checks of any kind, because it will be called only if it's needed

@xtuc
Copy link
Contributor

xtuc commented Jun 20, 2018

What would be your suggestion @viktor-ku? (I've might missed it from previous discussions). I can imagine a fixed list of extension or regexep but that won't be as generic as currently.

all loaders

We don't expect to have many loaders? (20 would already be a lot to me).

@viktor-ku
Copy link

@xtuc updated comment above

@xtuc
Copy link
Contributor

xtuc commented Jun 20, 2018

@viktor-ku one of my question was how do you call only the necessary loaders?

  • Based on a list of known extension? Limitating.
  • Based on provided regexp? Slow.

@viktor-ku
Copy link

@xtuc I guess you're right. It's slow. And I don't have any other ideas

@Janpot
Copy link

Janpot commented Jun 20, 2018

You could also decide to allow only one loader in node and write loaders in userland that can compose multiple loaders together.

@bmeck
Copy link
Member Author

bmeck commented Jun 20, 2018

@Janpot the problem with doing that is how composition works. If I have only a single loader how do I add to it. Imagine:

node --loader jspm index.mjs

And now I want to run my application with code coverage:

NODE_OPTIONS='--loader code-coverage'
node --loader jspm index.mjs

Use cases where you need loaders to coordinate are easy enough to show that we can't just have a single uncomposable loader. We could defer the composition to userland but that comes at the cost of not solving use cases with our default solution.

@Janpot
Copy link

Janpot commented Jun 20, 2018

Maybe you add the multiloader and push jspm deeper?

MULTILOADER_OPTIONS='code-coverage jspm'
node --loader multiloader index.mjs

But I see your point.

@bmeck
Copy link
Member Author

bmeck commented Jun 20, 2018

@Janpot possible, but that just devolves into everyone needing to use multiloader at which point we should just ship as minimally invasive form as is possible so that people could implement their own multiloader as part of the loader chain.

@Janpot
Copy link

Janpot commented Jun 20, 2018

Yes, I agree. I just remember seeing a comment somewhere that they wanted to do in core only what can't be done performantly in userland. But this would indeed take that a bit too far.

edit:

The main benefit I see in doing the multiloader is that you can push the cognitive overhead of figuring out the right behavior forward to after the initial release of ESM. At which point you can still implement the solution that the community converges too.

Then again, you could treat the whole implementation this way and push forward a lot of use-cases in this repo. Personally it would be the approach I'd go for, but I understand people want a complete implementation with sane defaults from the get go. (I don't want to create more noise in this issue as I'm not a member so you can safely ignore this philosophical side-track.)

@SMotaal
Copy link

SMotaal commented Jul 19, 2018

@Janpot I too was very eager for the next iteration of --loader a which I was thinking would look more like --loader a --loader b --loader c because a can delegate to b or not, b to c, c to builtin, but the more I explore this the more I realize that compared to user land module loaders, a one-fits-all plugin system is really what would be required if node will take part in negotiating precedence.

@bmeck please chime in on the following, would be interested to hear from your own perspective since you obviously explored this far more deeply than most.

Unlike userland loaders, you will not be able to simply replace the system if your requirements cannot be met. Lastly, not to forget hard learned lessons, efforts for standards (or widely accepted loaders) have shown that people agree to disagree, and Node.js is not a framework, we all need it to play nice with everyone.

Don't get me wrong, I am all for multi-loaders, but, I think that choreography cannot be in the form of decree.

So I am growing more favourable to --loader root-loader which I choose to use because it has a rich ecosystem of loaders which it can be configured to use in a way that I like to configure those things.

Such a loader would use an express-like mechanism because it is very likely that the platform (including v8) has evolved to optimize this kind of chained calls. But, someone else might have a better infrastructure to chain loaders... at the end of the day this cannot be prescribed, but surely it can be facilitated and enhanced through native optimizations as they evolve.

Here is a very rough chaining algorithm which takes an array of functions, and the position where the next(…) argument is supplied and executes the chain allowing any function to optionally delegate to the next function in line:

/// The usual things that Node could help facilitate ///
const base = (options.base = `file://${options.cwd || ''}`);

/**
 * How you locate and import things in preload 
 * is where I think Node's facilitation is highly 
 * mandated to promote stability and prevent wasted 
 * performance that popular things almost always
 * suffer from as they age (kind of like people).
 */
export const resolvers = new Set();

/// The following is highly opinionated ////

// Chain to be created once on first resolve
let chain;

// Must be constant (internally assigned not directly referenced)
export const resolve = async (specifier, referrer = base, resolve) =>
  (chain ||
    (resolve && (chain = new Chain([...resolvers, resolve], 2))))(
      specifier,
      referrer,
  );

function Chain(ƒs, position, thisArg) {
  return (...args) => {
    const p = position >= 0 ? position : args.length;
    const i = ƒs[Symbol.iterator]();
    const n = (...args) =>
      n.ƒ.apply(
        thisArg,
        ((args[p] = ((n.ƒ = i.next().value) && n) || undefined), args),
      );
    n.ƒ = i.next().value;
    return n(...args);
  };
}

@devsnek
Copy link
Member

devsnek commented Jul 19, 2018

@SMotaal for reference, here's the current design for "chaining" nodejs/node#18914

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants