Repository navigation
esm: The getPackageType utility function is not exposed #30514
Description
Activity
@nodejs/modules-active-members
Reacted by Derek LewisI'm against exposing machinery like this. We are planning to expose functions to let you call up to the previous loader.
I think this is also a really good example of why we shouldn't be adding new fields to package.json but oh well
Package type does not imply the module format, but rather the module format database, so note that the usage example is wrong as it doesn't support eg
.cjs.The source of truth to follow for determining the module format of a file is the specification for the resolver (https://nodejs.org/dist/latest-v13.x/docs/api/esm.html#esm_resolver_algorithm), under the function
ESM_FORMAT.Exposing or having userland implementations of the spec would be very useful indeed. If Node core were to expose something here it should likely be a JS
esmFormatimplementation. Then how to handle caching and whether it should be sync or async come up. Both have pros and cons. As you can see that is not necessarily trivial from a development and consensus perspective. Contributors to this process welcome! Userland work would likely be a great starting point.Does Node already do the
package.jsonlookup before this loader is called? Presumably it must have, I would think, to know how to reach this file (such as if it were referenced via"exports"). If so, it would both improve performance and solve this particular need if Node could pass along thepackage.jsontype (or allpackage.jsonmetadata) to the loader, so that the loader doesn’t need to look the data up again.I think exposing something is likely useful here since it is possible to sniff / can be looked up on disk and there isn't a clear workaround to make it easy to deal with. The main contention here for myself is what to expose; as @guybedford points out, just exposing the
"type"might not be the best choice as it doesn't really expose the underlying mechanisms that the runtime is using when that is set and could lead to misunderstanding and misuse.To give a concrete use case, I’m considering creating a CoffeeScript loader. So for a given
file.coffeethat this loader would process, I need to know the package type the same as if the file wasfile.js(since for CoffeeScript, we can’t store the parse goal metadata in the file extension and therefore I need to know the nearest parentpackage.json"type").So for me, I’d want to know either what Node would determine the package scope type to be if my file were a
.jsfile; or I’d want Node to provide to the loader the contents of the controllingpackage.jsonfile.I feel like this points to an inherent problem that tooling needs to know way too much about the specific details of node's esm resolution system. In core we've been really careful to avoid adding new stuff like that to the cjs loader because of the burden on the ecosystem to support it and the node maintainers to keep it working for the ecosystem.
even if we expose all the needed APIs, that doesn't guarantee tooling uses them, or uses them at the correct times (matching the behaviour of bugs is important too).
@GeoffreyBooth that sounds like you need to know the format of a file, not the package type.
@GeoffreyBooth that sounds like you need to know the format of a file, not the package type.
What do you mean? The format is CoffeeScript.
tooling needs to know way too much
Well, why does the loader API need to be given the type? Ideally that would be optional, and if it’s unspecified the regular Node logic would apply; and unknown types (e.g.
'.coffee') could be treated the same as.js, or I would have some way of telling Node to treat them like.js. And by “treat as.js” I mean, use the same logic that Node’s ESM loader uses to determine whether to treat.jsas CommonJS or as ESM.I’d want to know either what Node would determine the package scope type to be if my file were a .js file; or I’d want Node to provide to the loader the contents of the controlling package.json file.
As you stated, you want to know what format a
.jsfile is, not specifically the string value of the"type"field. Anyways, "package scope type" isn't done per file; a package has a set of formats mapping to file extensions within its boundaries. So, the package scope type of a file doesn't really make sense. You can see what format a package scope would treat a file to be though.Here is an implementation for getPackageType:
function getPackageType (checkPath) { const rootSeparatorIndex = checkPath.indexOf(path.sep); let separatorIndex; while ( (separatorIndex = checkPath.lastIndexOf(path.sep)) > rootSeparatorIndex ) { checkPath = checkPath.slice(0, separatorIndex); if (checkPath.endsWith(path.sep + 'node_modules')) return 'commonjs'; try { const pjson = JSON.parse(fs.readFileSync(checkPath)); return pjson.type || 'commonjs'; } catch (e) { if (e.code === 'ENOENT') continue; throw e; } } return 'commonjs'; }
This follows the specification (which is fully provided in the Node.js documentation exactly for this reason).
Because it is specified and relatively stable, there is nothing wrong with copy-pasting such a snippet. DRY does not apply when there are lots of decisions like API / async or async / caching concerns. Copy paste is good for this stuff. Or a userland packages with this code would be useful. Please be useful.
Reacted by Jan Olaf Martin and Geoffrey BoothThanks @guybedford, that certainly solves the need; though if this needs to happen for many/most loaders we might want to consider providing this as a utility, or reusing Node’s lookup rather than having this happen twice for each file.
This also makes me think of nodejs/modules#436; I wasn’t aware we treated a
node_modulesfolder as a special case. It shouldn’t need to be, as every subfolder ofnode_modulesshould have apackage.json; and we’re not really documenting this in the main docs, just in the resolver spec. I get that this is probably to cover some weird edge cases of files innode_modulesthat aren’t in subfolders, or something like that, but I wonder what the harm would be in removing this special casing.Reacted by Derek Lewisfolders within node_modules aren't guaranteed to have package.json, that's just a side effect of package managers. it's really important to highlight that prior to our esm loader, package.json didn't infer a "boundary" to node.
Reacted by Richard Lau and Jordan Harband/to @guybedford
Package type does not imply the module format, but rather the module format database, so note that the usage example is wrong as it doesn't support eg .cjs.
The goal of the example provided was to support use-cases like @GeoffreyBooth's, which don't use file extensions to determine module format; sorry, I should've clarified.
This follows the specification (which is fully provided in the Node.js documentation exactly for this reason).
Although the specification in the documentation may not need
GET_PACKAGE_TYPEto accomplish its goal, providing pseudocode for it might be useful to those implementing it in other languages. It looks like your implementation is a simplification ofESM_FORMAT,READ_PACKAGE_JSON,READ_PACKAGE_SCOPE./to @GeoffreyBooth
I get that this is probably to cover some weird edge cases of files in node_modules that aren’t in subfolders, or something like that,
I will provide an example of a project directory structure that currently uses this pattern. Since it's more relevant to the other issue I opened, I'll put it in there.
but I wonder what the harm would be in removing this special casing.
Ditto. I'm guessing that it would mean that there would need to be a
package.jsonin there instead./to @devsnek
it's really important to highlight that prior to our esm loader, package.json didn't infer a "boundary" to node.
There are actually two boundaries now:
package.jsonandnode_modules.14 remaining items
I just played around with the resolve hook a bit. The third parameter is the default ES resolver. I tried writing a loader like this:
export async function resolve(specifier, parentModuleUrl, defaultResolver) { console.log(defaultResolver(specifier));
And I ran it via
node --experimental-loader ./loader.js test/index.coffee, where the current working folder contained apackage.jsonwith"type": "module". It printed this (url shortened):{ url: 'file:///app/loader/test/index.coffee', format: 'module' }
I put a
package.jsonwith{"type": "commonjs"}intest/and ran it again, and got the same output but withformat: 'commonjs'. This would seem to solve the need at the top of this thread. I guess the default ES resolver treats unknown extensions the same as.jsfor the purposes of determining format/type?Rewriting the specifier to something that doesn’t exist, like replacing
.coffeewith.js, throwsERR_MODULE_NOT_FOUND; so if there’s a use case for querying the type of an existing path to a nonexistent file, then that use case is still unsatisfied.@GeoffreyBooth it seems you've found a bug actually. I've posted #30632. Following the specification, there is supposed to be an unsupported module format error thrown for this case when "type": "module" is set. It is supposed to support format: 'commonjs' in the non module case though.
Okay. Well as far as I understand this
resolvehook, it’s supposed to return an object like:{ url: 'file:///app/loader/test/index.coffee', format: 'module' }
Could something in this return object tell the loader to do the “determine the package type as if
urlwas a.jsfile” logic? Like leaving outformat, or setting it toautoor some other special string?That way we wouldn’t need to expose this utility method, and potentially cause this logic to run twice for every specifier, but custom loaders could still take advantage of this internal method.
Also, it's problematic that a resolve hook would return a format. It assumes that at URL resolution time the content-type behind a URL is already known. I would hope that a final loader hook API only returns that information when actually fetching the resource contents.
Reacted by Derek Lewis/to @jkrems
Also, it's problematic that a resolve hook would return a format. It assumes that at URL resolution time the content-type behind a URL is already known. I would hope that a final loader hook API only returns that information when actually fetching the resource contents.
I second this hope and I wonder if we share the same concern. With the current API, if the loader needs any of the source contents to determine the module format (e.g. @module JSDoc and containsModuleSyntax), it will end up reading file contents twice (once when doing the parsing of the format in the loader, and another when being added to the cache internally).
/to @GeoffreyBooth
Speaking of which, where's the hook for transforming the source? Is that just not implemented yet?
It seems like the dynamic instantiate hook can be leveraged for that purpose. No guarantees, but I'll try to put together a demo as a proof of concept. I've been looking for a reason to get familiar with this hook. Please let me know if you find success before I do. 😃
/to @guybedford
If Node core were to expose something here it should likely be a JS esmFormat implementation. [...] Contributors to this process welcome! Userland work would likely be a great starting point.
Here are the three functions associated with
esmFormatimplemented in userland JS. They have the specification inlined as comments and have been typechecked with the Closure and TypeScript compilers. Sorry for the delay, I wanted to use them for a bit to make sure they are working as expected.Userland JS implementation of Node's ESM resolver spec
I will also be doing the remaining three and write unit tests for them. Hope this helps! I must know how we can better collaborate. Where would these go in the Node core repo? I need more guidance about contributing things like this.
Absolutely amazing work to see @DerekNonGeneric! We discussed having an open test suite for the resolver at the last meeting, if you want to work with core to get the unit tests made as a suite that can be tested against userland resolvers as well that could be an interesting effort, but not pressure on that either.
Just having those functions available to users spec compatible as a resource to install or fork in their own apps unlocks a huge amount of value.
Reacted by Derek LewisI'd still like to try and decouple determining format/metadata from determining location. There are a few ways we could do that, but if we do go with an API like
loaderMetaData(url: String<URL>)I'd want the return type to be extensible and to ensure theurldoes not need to exist. Such an seems a bit more than the original discussion of this issue though and perhaps it would be good to expose a different issue for each of these APIs and a tracking/coordination issue. The original issue here to me at least seems to be solved byloaderFileFormatMappingWithin(url: DirName<String<URL>>)that fails for non-file:URL schemes.The original issue here to me at least seems to be solved by
loaderFileFormatMappingWithin(url: DirName<String<URL>>)that fails for non-file:URL schemes.@bmeck, did you mean to say would be solved? (Unclear if you're proposing a solution or stating that one already exists.)
I've removed the modules agenda label here as this has been discussed a number of times now. If anyone wants to re-add though feel free.
@guybedford, what was the conclusion?
We just merged #30986 which I think makes the need for this function unnecessary. Within
resolveyou no longer need to returnformat, and insidegetFormatyou can do this:// Ask Node to determine the format as if the current URL had a .js extension return defaultGetFormat(`${url}.js`);
@GeoffreyBooth, sounds good to me.
Build tooling in need of this feature, please refer to #49446.
Is your feature request related to a problem? Please describe.
Assist in determining module format by using internal machinery.
Describe the solution you'd like
Expose the
getPackageTypeutility function.Describe alternatives you've considered
The alternative is what I am currently doing, which is passing the
--expose-internalsflag (no bueno) and acquiring the utility function viainternalBinding('module_wrap'). Exposing it would allow shaving off one dangerous flag and five lines from the following example.As you may have noticed, one use-case is for custom loaders, which necessitate this info. Amongst others, one advantage of using the internal machinery (over writing it in JS) is that it is implemented in C++, which is important because this particular algorithm can be very expensive to perform and ( ! ) resolve hooks are on the hot code path of every module.
Additionally, exposing this function may also improve the viability of using loaders to fill the current gap in format autodetection until the
--es-module-resolutionflag's proposedautomode is ready.cc @nodejs/modules-active-members
bcc @GeoffreyBooth (please cc above)