esm: The getPackageType utility function is not exposed

@GeoffreyBooth

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 getPackageType utility function.

Describe alternatives you've considered
The alternative is what I am currently doing, which is passing the --expose-internals flag (no bueno) and acquiring the utility function via internalBinding('module_wrap'). Exposing it would allow shaving off one dangerous flag and five lines from the following example.

// Flags: --expose-internals --experimental-modules --experimental-loader

import { createRequire /*, getPackageType*/ } from 'module';
import { dirname as pathDirname } from 'path';
import { fileURLToPath as pathFromFileUrl } from 'url';

const __filename = pathFromFileUrl(import.meta.url);
const __dirname = pathDirname(__filename);
const require = createRequire(__dirname);
const { internalBinding } = require('internal/test/binding');
const { getPackageType } = internalBinding('module_wrap');

// const TYPE_NONE = 0;
// const TYPE_COMMONJS = 1;
const TYPE_MODULE = 2;

/**
 * @param {string} specifier
 * @returns {Promise<{url: string, format: string}>}
 */
export async function resolve(specifier) {
  const type = getPackageType(specifier);
  if (type === TYPE_MODULE) {
    return {
      url: specifier,
      format: 'module',
    };
  }
}

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-resolution flag's proposed auto mode is ready.

cc @nodejs/modules-active-members
bcc @GeoffreyBooth (please cc above)

@nodejs/modules-active-members

I'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 esmFormat implementation. 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.json lookup 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 the package.json type (or all package.json metadata) to the loader, so that the loader doesn’t need to look the data up again.

@guybedford

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.coffee that this loader would process, I need to know the package type the same as if the file was file.js (since for CoffeeScript, we can’t store the parse goal metadata in the file extension and therefore I need to know the nearest parent package.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 .js file; or I’d want Node to provide to the loader the contents of the controlling package.json file.

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

@GeoffreyBooth that sounds like you need to know the format of a file, not the package type.

@GeoffreyBooth

@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 .js as CommonJS or as ESM.

@GeoffreyBooth

@GeoffreyBooth

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 .js file 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.

@guybedford

Thanks @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_modules folder as a special case. It shouldn’t need to be, as every subfolder of node_modules should have a package.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 in node_modules that aren’t in subfolders, or something like that, but I wonder what the harm would be in removing this special casing.

folders 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.

@guybedford

/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_TYPE to 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 of ESM_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.json in 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.json and node_modules.

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 a package.json with "type": "module". It printed this (url shortened):

{
  url: 'file:///app/loader/test/index.coffee',
  format: 'module'
}

I put a package.json with {"type": "commonjs"} in test/ and ran it again, and got the same output but with format: '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 .js for the purposes of determining format/type?

Rewriting the specifier to something that doesn’t exist, like replacing .coffee with .js, throws ERR_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

@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 resolve hook, 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 url was a .js file” logic? Like leaving out format, or setting it to auto or 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.

@bmeck

I would hope that a final loader hook API only returns that information when actually fetching the resource contents.

Speaking of which, where's the hook for transforming the source? Is that just not implemented yet? @bmeck @jkrems @a-lxe

@GeoffreyBooth

/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 esmFormat implemented 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.

@DerekNonGeneric

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.

I'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 the url does 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 by loaderFileFormatMappingWithin(url: DirName<String<URL>>) that fails for non-file: URL schemes.

@bmeck

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

@guybedford, what was the conclusion?

We just merged #30986 which I think makes the need for this function unnecessary. Within resolve you no longer need to return format, and inside getFormat you can do this:

// Ask Node to determine the format as if the current URL had a .js extension
return defaultGetFormat(`${url}.js`);

@GeoffreyBooth

@GeoffreyBooth, sounds good to me.

Build tooling in need of this feature, please refer to #49446.

Navigation Menu

Repository navigation

esm: The `getPackageType` utility function is not exposed #30514

14 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

esm: The getPackageType utility function is not exposed #30514

Description

Activity

GeoffreyBooth commented on Nov 17, 2019

devsnek commented on Nov 17, 2019

guybedford commented on Nov 17, 2019

GeoffreyBooth commented on Nov 18, 2019

bmeck commented on Nov 18, 2019

GeoffreyBooth commented on Nov 18, 2019

devsnek commented on Nov 18, 2019

bmeck commented on Nov 18, 2019

GeoffreyBooth commented on Nov 18, 2019

bmeck commented on Nov 18, 2019

guybedford commented on Nov 18, 2019

GeoffreyBooth commented on Nov 18, 2019

devsnek commented on Nov 18, 2019

DerekNonGeneric commented on Nov 19, 2019

14 remaining items

GeoffreyBooth commented on Nov 25, 2019

guybedford commented on Nov 25, 2019

GeoffreyBooth commented on Nov 25, 2019

hybrist commented on Nov 25, 2019

GeoffreyBooth commented on Nov 25, 2019

DerekNonGeneric commented on Nov 26, 2019

guybedford commented on Nov 27, 2019

bmeck commented on Dec 6, 2019

DerekNonGeneric commented on Dec 7, 2019

guybedford commented on Jan 7, 2020

DerekNonGeneric commented on Jan 7, 2020

GeoffreyBooth commented on Jan 7, 2020

DerekNonGeneric commented on Jan 7, 2020

DerekNonGeneric commented on Jan 7, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

esm: The `getPackageType` utility function is not exposed #30514