Skip to content
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

doc: READ_PACKAGE_SCOPE returns JSON not a URL #40578

Closed
1 task
evanw opened this issue Oct 23, 2021 · 2 comments
Closed
1 task

doc: READ_PACKAGE_SCOPE returns JSON not a URL #40578

evanw opened this issue Oct 23, 2021 · 2 comments
Labels
doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation.

Comments

@evanw
Copy link

evanw commented Oct 23, 2021

📗 API Reference Docs Problem

  • Version: v16.11.1
  • Platform: Darwin
  • Subsystem: esm

Location

Resolver Algorithm Specification

Affected URL(s):

Description

In the resolver algorithm specification, READ_PACKAGE_SCOPE returns a parsed JSON value but is then used as a URL. Surely this is a mistake. In detail:

  1. The READ_PACKAGE_JSON function either returns null, throws an error, or returns a parsed JSON value:

    READ_PACKAGE_JSON(packageURL)

    1. Let pjsonURL be the resolution of "package.json" within packageURL.
    2. If the file at pjsonURL does not exist, then
      1. Return null.
    3. If the file at packageURL does not parse as valid JSON, then
      1. Throw an Invalid Package Configuration error.
    4. Return the parsed JSON source of the file at pjsonURL.
  2. The READ_PACKAGE_SCOPE function either returns null or the result of calling READ_PACKAGE_JSON, so it also returns a parsed JSON value (called pjson below):

    READ_PACKAGE_SCOPE(url)

    1. Let scopeURL be url.
    2. While scopeURL is not the file system root,
      1. Set scopeURL to the parent URL of scopeURL.
      2. If scopeURL ends in a "node_modules" path segment, return null.
      3. Let pjson be the result of READ_PACKAGE_JSON(scopeURL).
      4. If pjson is not null, then
        1. Return pjson.
    3. Return null.
  3. The ESM_FORMAT function calls READ_PACKAGE_SCOPE and uses the result as a parsed JSON object (called pjson below):

    ESM_FORMAT(url)

    1. Assert: url corresponds to an existing file.
    2. Let pjson be the result of READ_PACKAGE_SCOPE(url).
    3. If url ends in ".mjs", then
      1. Return "module".
    4. If url ends in ".cjs", then
      1. Return "commonjs".
    5. If pjson?.type exists and is "module", then
      1. If url ends in ".js", then
        1. Return "module".
      2. Throw an Unsupported File Extension error.
    6. Otherwise,
      1. Throw an Unsupported File Extension error.
  4. However, both PACKAGE_SELF_RESOLVE and PACKAGE_IMPORTS_RESOLVE call READ_PACKAGE_SCOPE and use the result as a URL (called packageURL below). In particular, they pass the result to READ_PACKAGE_JSON again. I assume READ_PACKAGE_JSON is not meant to be called on its own result:

    PACKAGE_SELF_RESOLVE(packageName, packageSubpath, parentURL)

    1. Let packageURL be the result of READ_PACKAGE_SCOPE(parentURL).
    2. If packageURL is null, then
      1. Return undefined.
    3. Let pjson be the result of READ_PACKAGE_JSON(packageURL).
    4. If pjson is null or if pjson.exports is null or
      undefined, then
      1. Return undefined.
    5. If pjson.name is equal to packageName, then
      1. Return the result of PACKAGE_EXPORTS_RESOLVE(packageURL,
        packageSubpath, pjson.exports, defaultConditions).
    6. Otherwise, return undefined.

    PACKAGE_IMPORTS_RESOLVE(specifier, parentURL, conditions)

    1. Assert: specifier begins with "#".
    2. If specifier is exactly equal to "#" or starts with "#/", then
      1. Throw an Invalid Module Specifier error.
    3. Let packageURL be the result of READ_PACKAGE_SCOPE(parentURL).
    4. If packageURL is not null, then
      1. Let pjson be the result of READ_PACKAGE_JSON(packageURL).
      2. If pjson.imports is a non-null Object, then
        1. Let resolved be the result of
          PACKAGE_IMPORTS_EXPORTS_RESOLVE(
          specifier, pjson.imports, packageURL, true, conditions).
        2. If resolved is not null or undefined, return resolved.
    5. Throw a Package Import Not Defined error.

If this was type checked, this would be a type error. I'm not sure what the right fix is. Maybe READ_PACKAGE_SCOPE should return a URL instead? Anyway I think the docs should be updated even if it's possible to figure out what they actually mean, because the mismatch is unnecessarily confusing.


  • I would like to work on this issue and
    submit a pull request.
@evanw evanw added the doc Issues and PRs related to the documentations. label Oct 23, 2021
@Mesteery Mesteery added the esm Issues and PRs related to the ECMAScript Modules implementation. label Oct 23, 2021
@targos
Copy link
Member

targos commented Oct 24, 2021

/cc @guybedford

@guybedford
Copy link
Contributor

Fix in #40592.

targos pushed a commit that referenced this issue Nov 6, 2021
PR-URL: #40592
Fixes: #40578
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
danielleadams pushed a commit that referenced this issue Jan 29, 2022
PR-URL: #40592
Fixes: #40578
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants