-
Notifications
You must be signed in to change notification settings - Fork 17
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
Proposal: Rename “loaders” #95
Comments
I think this sounds like a sound idea. It enormously expands our scope/purview though. |
Well I think those other things were already part of our scope 😄 just informally so. |
cc @nodejs/tsc for awareness and permission. I can give an update at the next TSC meeting. |
I'm +1 on a terminology change, but I'm not sure we can use |
Ah, good shout. I definitely don't want to be responsible for asnyc_hooks 😵💫 |
Should definitely consider a single CLI flag that is empowered to make all necessary runtime customizations. It fits the goals of nodejs/node#43818 and nodejs/node#43408: runtime customization with good user-experience, acknowledging that TS support (and other stuff) requires more than just loader hooks. Splitting hook registration mechanisms across multiple CLI flags feels like bad user experience. |
Then "Loader hooks" doesn't make sense because many of the types of customization people want to do don't involve loaders. |
Hrrm, I think we can't use I don't love |
|
Hooks aren't inherently customization. Async hooks and performance hooks aren't about customizing Node. Also the hooks are registered during startup but they apply to the lifetime of the process. I don't see them as related to startup. |
Naming things--the hardest problem in computer science. 😅 Seriously though, I think the current "loaders" naming is fine until we actually specify how the system is expanding. The eventual shape of this subsystem is very nebulous. I don't think the right name for it will be clear until that lack of scope clarity is resolved. |
I think we know how it’s expanding; I wrote a list in the top post. We don’t need to map out the specifics of each of those items to know that we’re essentially creating opportunities for users to give Node functions to call at critical points in Node’s internal flows, where those functions change Node’s behavior in some desirable way. Historically, customization of Node’s behavior has been done via monkey-patching; CommonJS made it possible to override publicly accessible Node APIs with custom versions, and so that became the go-to (and for many things, only) way to customize. ESM put a stop to that, not because the Node team wanted to clamp down on this but because the ESM spec itself meant that all the code in an application needed to be resolved and parsed before any of it was executed, and so therefore any code that attempted to monkey-patch would be “too late” in that what it’s attempting to override would have already been loaded. Hence loaders were born, initially focused on restoring this lost monkey-patching ability. As time goes on, though, it’s become clear that monkey-patching—whether the CommonJS version or the ESM loaders version—is a blunt tool. It’s better to provide intentional APIs for specific purposes, like So in my mind, the current loaders API is really a “register these functions to alter Node’s behavior at specified points” API. And as time goes on we’ll be adding support for more named functions beyond @mcollina I don’t know if we’re ever likely to need an API namespace like Somewhat related is this issue for Deno someday adding support for types of transpilation other than TypeScript: denoland/deno#1739. They just call it a “public API,” which is perhaps too generic. You could also argue that maybe these functions are essentially plugins, and so what we’re really providing here is a Node Plugins API. (Or middleware, or injected functions. . . .) Another source of inspiration that feels similar to me is WordPress Hooks: https://developer.wordpress.org/plugins/hooks/. They work actually quite similarly to our hooks, and there’s a lot of language on that page that could provide ideas for names:
|
It sounds like you're trying to adapt loaders to do what diagnostics_channel already does. APMs are already using diagnostics_channel to replace monkey-patching. Our use of loaders is somewhat a temporary hack to fill the gap while transitioning the ecosystem away from needing monkey-patching to in the future supporting diagnostics_channel events. The main difference between that and what you propose is the addition of being able to modify behaviour, which was explicitly avoided by diagnostics_channel for security and stability reasons. It was our assessment that it made more sense to make more narrowly-focused APIs in any places that needed to change behaviour. Node.js is full of years of attempts at all-encompassing systems collapsing under their own weight and becoming unmaintainable to all but the extremely dedicated. Domains is one example, async_hooks is another, and AsyncListener before that. I would focus on stabilizing loader hooks before considering expanding the scope even further to include an even broader collection of things, which will likely drag on experimental status indefinitely as there will always be more use cases. If you still intend to kick off other efforts I would intentionally bucket them separately, even if they're connected features, just to keep focused scope around each effort and have separate paths for stabilization of each so they're not cross-dependent in ways that could potentially deadlock progress in the future. |
Agree on this. But before making loaders non-experimental, we need to pick a name. If it’s going to be something other than the Loaders API, then the Earlier today I landed #97 to update our roadmap: https://github.com/nodejs/loaders#status. I divided our to-do list into two phases based on what I thought was needed for us to consider ESM to be at parity with CommonJS, which was the point of the loaders API. We’re pretty close to that milestone (the first set of tasks). We could possibly move this API from experimental to stable at that point, assuming we can settle on a permanent name. |
I don't see why the flag would need to change. It's for defining loaders. What we're talking about is something entirely different. |
For what we currently call "loaders", what about
The name Then for subsequent hooks, be they fs hooks, or what-have-you, cross those bridges when we come to them? Or, are we thinking we might want to glob all the hooks in the same file? eg if we ever add fs hooks, they would live in the same file as ESM hooks? export async function resolve() {}
export async function load() {}
export async function readFile() {}
// … If that's the case, I think the flag should merely be Our team name could be "Hooks" or something—collaborators would all know that we handle all the hooks except |
Yes, this was my thinking. That file could import other files, but ultimately the API is this top-level file that exports functions of defined names. It makes sense to group them all together because something like TypeScript will use many of them. |
@GeoffreyBooth a bit off topic but is nodejs/node#41076 the current status for file system hooks or are there other issues I should go looking for? |
Yes, as far as I’m aware. @arcanis can confirm. |
We could also maybe have a flag name like Then the API itself could be named something more verbose, whether Customization Hooks or Plugin Hooks or Process Hooks (because they affect the Node process) or whatever else. Or just Hooks if everyone is okay with that. |
For now I've paused #41076 to first see whether the helper API, together with |
This is related to user experience. If those hooks require a different CLI flag, then |
I'm not convinced renaming the |
I like the idea of a Avoids the double-execution, avoids the need for users to pass multiple CLI flags, allows programmatic bootstrapping in the main thread, so it allows config file resolution and whatnot. |
In the recent loaders meeting, @aduh95 pointed out that once loaders move off-thread, we can’t have a single user file like The docs describe all of Node’s flags and supported environment variables in a section called “Command-line API.” I think once we build support for some way of specifying those options via a config file (#98) that section should probably be renamed something like Configuration API and include flags, environment variables and config files. Then we could create a new section called Customization API with subsections Module Hooks, REPL Hooks, Source Maps Hooks and so on, for all the parts of Node where we would support customization via user-supplied functions. This team could become the Customization Team, to include the scope of customization across Node, or the Module Hooks team to stay focused on customization particular to the module systems (CommonJS and ESM). Thoughts? |
The main thread hooks should still be able to programmatically register and configure loader hooks on the user's behalf. Or rather, a single declaration should be all that's needed for a user to opt-in to a suite of hooks. Otherwise user experience is bad in the "one-stop shop" use-case. (#95 (comment)) |
If this is possible, by all means, sure. The “two flags” approach is only if we can’t get a single flag to work. Anyway, what about the rest? I’d like to wrap up this discussion and reach a decision.
What do you all think? |
Just a nit, but in my mind, the two flags will always exist, because they serve an important use-case: avoiding unnecessary execution in the main thread. But it should also be possible to apply both kinds of hooks with a single flag. Not sure if we already agree on that or if it is worth clarifying. |
Could we perhaps resolve this discussion without needing a meeting? Slight amendment to my last comment per #105 (comment):
Please reply with:
|
Last call for votes ☝️ otherwise I think we have a winner. |
Closing per #95 (comment). I think we can consider the next steps as listed in #95 (comment) to be approved. I’ll try to get around to the administrative ones (repos, etc.) soon; if anyone wants to tackle the docs PR, please drop a comment here letting everyone know you want to work on it. |
In many of the discussions on this repo and in https://github.com/nodejs/node, we’ve considered various parts of Node that users might want to customize beyond just module resolution and loading. Within this repo one major use case discussed has been virtual filesystems, which would require hooking into many (if not all) of the
fs
methods. In nodejs/node#43818 (comment) @cspotcode has compiled a great (incomplete) list of places where someone adding support for TypeScript to Node might want to customize Node:I’m thinking perhaps a better name for this repo and the Loaders API itself would be “hooks” / Hooks API. This would better reflect even the existing API, since arguably
globalPreload
has little to do with module resolution and loading. We could also move the loaders documentation out of the ESM page in the docs and into its own page. cc @nodejs/loadersIf the current loaders team likes this idea, I can open an issue on nodejs/admin to make it happen from a technical standpoint.
Update per #95 (comment), here are the new names:
The text was updated successfully, but these errors were encountered: