ClojureScript

:npm-deps using ES6 modules with .mjs extensions are not detected correctly

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: 1.10.238
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:
    Tested with both ClojureScript 1.10.64 and ClojureScript master (4375a63)

Description

Several modern JS libraries have adopted the .mjs extension for ES6 modules. Two examples of such libraries are https://github.com/leebyron/iterall/ and https://github.com/graphql/graphql-js/.

Indexing packages that use this file extension (e.g. for their index.mjs) and adding them to the compiler environment is currently broken. As a result, these packages will not be resolvable at runtime, leading to errors such as this:

base.js:1357 Uncaught Error: Undefined nameToPath for iterall
    at visitNode (base.js:1357)
    at Object.goog.writeScripts_ (base.js:1369)
    at Object.goog.require (base.js:706)
    at index.html:7

I've created a minimal repository to reproduce the issue here: https://github.com/Jannis/cljs-npm-deps-mjs-issue.

I'll attach a patch with a fix and two tests (one to verify indexing, one to verify building) shortly.

  1. CLJS-2592.3.patch
    09/Mar/18 12:14 PM
    13 kB
    Jannis Pohlmann
  2. CLJS-2592.4.patch
    10/Mar/18 5:33 AM
    12 kB
    Jannis Pohlmann
  3. CLJS-2592.patch
    02/Mar/18 8:47 AM
    7 kB
    Jannis Pohlmann
  4. CLJS-2592.patch
    02/Mar/18 8:40 AM
    7 kB
    Jannis Pohlmann

Activity

Hide
Jannis Pohlmann added a comment -

I realise this is probably not specific to {:npm-deps} at all and would affect any way in which foreign libs are defined?

Show
Jannis Pohlmann added a comment - I realise this is probably not specific to {:npm-deps} at all and would affect any way in which foreign libs are defined?
Hide
Jannis Pohlmann added a comment -

Patch that fixes the issue and adds two tests (to verify indexing and to verify build output).

I can confirm that the repo to reproduce the issue works fine when using a local build of ClojureScript after applying the patch.

Show
Jannis Pohlmann added a comment - Patch that fixes the issue and adds two tests (to verify indexing and to verify build output). I can confirm that the repo to reproduce the issue works fine when using a local build of ClojureScript after applying the patch.
Hide
Jannis Pohlmann added a comment -

Updated patch with tests renamed to mention this JIRA issue.

Show
Jannis Pohlmann added a comment - Updated patch with tests renamed to mention this JIRA issue.
Hide
Jannis Pohlmann added a comment -

Another update: as much as I'd like to see this fixed, the provided patch only works with simple JS packages like iterall. It doesn't work with e.g. graphql yet, which has many .mjs files that reference each other, and also has .js files parallel to that.

Show
Jannis Pohlmann added a comment - Another update: as much as I'd like to see this fixed, the provided patch only works with simple JS packages like iterall. It doesn't work with e.g. graphql yet, which has many .mjs files that reference each other, and also has .js files parallel to that.
Hide
Jannis Pohlmann added a comment -

Updated patch that adds an optional :package-json-resolution flag (supported values: :webpack, :nodejs, ["foo", "bar", ...].

Show
Jannis Pohlmann added a comment - Updated patch that adds an optional :package-json-resolution flag (supported values: :webpack, :nodejs, ["foo", "bar", ...].
Hide
Jannis Pohlmann added a comment -

Revised version of the previous patch that doesn't introduce a :browser target.

Show
Jannis Pohlmann added a comment - Revised version of the previous patch that doesn't introduce a :browser target.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: