incorrect path generated for module in node_modules.

[trusktr]

Hello! This is a great project.

I'm getting this error on my webapp:

GET https://my-site.foo/dist/node_modules/lume/node_modules/lowclass/dist/index.js 404

But it appears that this URL is wrong, because npm flattens dependencies, so that dependency is actually located at

https://my-site.foo/dist/node_modules/lowclass/dist/index.js 

in the top level of node_modules. I get many such errors, where the dependency is not nested, but at the top level.

Any plans to fix this? Or is there a workaround?

[dmail]

Hi,

I don't think it comes from this repository because it's used a lot in various scenarios and there is unit tests covering your situation.

I suspect your file structure and importmap are out of sync: Have you regenerated the importmap recently ? Because after every npm install npm might have moved some node modules. To ensure importmap and node_modules directory are in sync you must regenerate the import map after every npm install.

[dmail]

I have added a unit test for this issue.

https://github.com/jsenv/jsenv-node-module-import-map/blob/d023a77ff31cd0760ced046ab37e0326b13305c0/test/getImportMapFromNodeModules/core/issue-20/issue-20.test.js#L33-L40

As shown in the test case the generated importmap resolves to /node_modules/lowclass/dist/index.js. In your case the browser is performing a request to /node_modules/lume/node_modules/lowclass/dist/index.js instead. It might be because:

• the importmap you use is not in sync with your filesystem and should be generated again (what I suggest in my previous comment).
• Your node_modules file structure generates an importmap that is wrong (there is an issue in the source code of this repository)
• what you use to resolve url using importmap has an issue
• my way of resolving url using importmap has an issue

[dmail]

By pushing https://github.com/jsenv/jsenv-node-module-import-map/commit/ff5c0bc1a2f3cef990f5e8607795fe160295d45e I can break the unit test.

But let's say you have the following file structure after npm install:

node_modules/
  lowclass/
    dist/
      index.js
  lume/
    node_modules/
      lowclass/
        dist/
          index.js

In this scenario the generated importmap will look like this (I keep only the remapping for "lowclass")

{
  "imports": {},
  "scopes": {
    "lume": {
      "lowclass": "./node_modules/lume/node_modules/lowclass/dist/index.js"
    }
  }
}

This is because in that file structure there is a node_modules/lume/node_modules/lowclass. node_modules/lowclass cannot be used by node_modules/lume because it's certainly incompatible with requirement in node_modules/lume/package.json. In other words node_modules/lowclass/package.json version is different from node_modules/lume/node_modules/lowclass/package.json. Npm keep both certainly because they are not required by the same module.

In that context the importmap generated by this repository is correct as it favors node_modules/lume/node_modules/lowclass over node_modules/lowclass just like Node.js do.

[trusktr]

@dmail Thanks for the quick effort on this!

Here is a reproduction: https://gitpod.io/#https://github.com/trusktr/babel-decorator-test-1.git

Check out the start script in package.json which runs generate-import-map.mjs. The GitPod instance will open a browser preview after it automatically starts the project with npm start, then you can open console to see the error. You can always re-run npm start in the editor terminal. It's just like VS Code. On the left you can see the file explorer and see what's in node_modules.

You should see an error like this:

Error: 404  https://8080-df7664df-f156-45d2-aa0c-1d28c6394e09.ws-us02.gitpod.io/dist/node_modules/@lume/element/node_modules/@lume/variable/dist/index.js

then if you browse in the file explorer to node_modules/@lume/element/ you'll see there is not a node_modules folder there, but you can find that the folder node_modules/@lume/variable/ exists at the top level.

If you take a look at node_modules/@lume/element/package.json, you'll see it depends on "@lume/variable": "^0.1.0", and if you look in node_modules/@lume/variable/package.json you'll see that the version is 0.1.0, but the variable package is not located at node_modules/@lume/element/node_modules/@lume/variable because npm flattened the structure.

NPM always flattens unless there are incompatible versions, in which case it needs to keep the incompatible versions further down in the tree for each dependent that relies on the "forked" dependencies.

[dmail]

NPM always flattens unless there are incompatible versions, in which case it needs to keep the incompatible versions further down in the tree for each dependent that relies on the "forked" dependencies.

True, that's one of the reason why this package uses filesystem to find node modules and generate the importmap.

Here is a reproduction: https://gitpod.io/#https://github.com/trusktr/babel-decorator-test-1.git

I discover gitpod it's amazing :).

I followed your link but there is no generate-import-map.mjs file and package.json start script does not contain the command to generate import maps.

[trusktr]

I discover gitpod it's amazing :).

Yeah!

Sorry about that, I realized I pasted the wrong link. It is this one: https://gitpod.io/#/https://github.com/trusktr/lume-babel-starter. Then you can follow the same steps as before.

[dmail]

Hi thanks for the updated link. I managed to reproduce and there is two distinct issues:

1) es-module-shims scoped mapping resolution is different from what I understood in importmap specs

This is causing the issue you are mentioning and results into the 404 on dist/node_modules/@lume/element/node_modules/@lume/variable/dist/index.js. I have opened issue https://github.com/guybedford/es-module-shims/issues/90 to understand what is the correct behaviour to resolve scoped import. If you look into the issue you will find minimal step to reproduce that causes the 404.

If the issue comes from es-module-shims I will work with them to update it. If the issue comes from this repository I will update how scoped import are written in the importmap file to match es-module-shims and importmap specifications.

Frankly I think it's on me but let's wait for an answer to advise.

2) lume is using extensionless imports

There is more 404 on urls like dist/node_modules/@lume/element/dist/Element because lume is using extensionless imports. For instance one of the lume source file is transpiled by babel into

export * from '@lume/variable';
export * from './Element';
export * from './element-type-helpers';
export const version = '0.1.2';
//# sourceMappingURL=index.js.map

And the browser ends up doing a request missing the .js extension. To avoid this there is no perfect solution Ideally, lume or babel should generate file with the .js extension. Alternatively they could provide an importmap to remap extensionless import to import with an extension. I see you can override how es-module-shims fetches using https://github.com/guybedford/es-module-shims#fetch-hook. You could use that to add the extension when missing ?

[dmail]

guybedford/es-module-shims#90 issue is resolved, upgrading es-module-shims to 0.6.0 fix some of the 404 errors you are facing.

About extensionless import used by lume: They use TypeScript and they generate dist files using the following code: https://github.com/lume/cli/blob/01072ac2e64657f6db2c0b8e453cd6b895251fff/bin/commands.js#L47. This buildTs function is using tsc (typescript compiler) to transform typescript files into javaScript files. Unfortunately the typescript compiler does not append .js extension making javascript files unusable without an other tool. There is a 3 years old issue about it: https://github.com/microsoft/TypeScript/issues/16577.

Lume cli documentation states it can outputs standard esmodule format in https://github.com/lume/cli#current-features

Maybe you should open an issue on their side (in the lume cli repository) to ask them if they can add .js extension to the files generated in dist. Even if it's not yet in the TypeScript compiler they might have an other solution.

[trusktr]

Awesome! Thanks for finding that issue! :pray: Can't wait to try this!

Lume is my project, btw :), and "they" is just two of us right now :smile:. I'll figure how to solve the extensions thing. Looks like there's this nifty Babel plugin to do exactly that: https://www.npmjs.com/package/babel-plugin-add-import-extension

[trusktr]

The ecosystem is a bit fragmented at the moment.

If a 3rd-party dependency is in CommonJS format, or the 3rd-party dependency doesn't have .js in all specifiers, or the 3rd-party dependency has an exports field that TypeScript does not use for re-mapping, we're quite stuck.

Using Webpack or other similar tools is a lot easier at this point (for a one-day solution, compared to having to fork all dependencies and submitting PRs).

[dmail]

Thank you for the follow up. As you seems to be focusing on these problematics at the moment I'll try to give you more info:

About extension less import

There is an other issue talking about importmap not generated for extension less imports: https://github.com/jsenv/jsenv-node-module-import-map/issues/21.

To fix it, this repository could parse all the files in node_modules/** to find thoose extension less imports.

But:

• Parsing files takes lot of time
• Parsing files means supporting TypeScript and other cases where it's not really js

About exports field

There is a non documented packagesManualOverrides parameter (refered as manualOverrides in https://github.com/jsenv/jsenv-node-module-import-map/issues/16#issuecomment-613340636). Using packagesManualOverrides is a way to have the correct importmap without having to fork all the things.

About current behaviour of this repository

My past self decided to let library author the responsability to configure exports and to avoid extensionless imports (at least in dist files).

Any idea?

As you said: The ecosystem is a bit fragmented at the moment. Regarding that current situation, do you have an idea to improve this tool? (Maybe it's ok as it is).

[trusktr]

Hello! Finally swung back to this.

I'm using .js extensions now, so that's all good. However I get an error like

npm:es-module-shims@0.9.0:474 Uncaught (in promise) Error: Unable to resolve specifier '@lume/element' from http://localhost:5000/dist/Login.dom.js
    at throwUnresolved (npm:es-module-shims@0.9.0:474)

Are you able to install @lume/element, gen import maps, and import it?

[dmail]

Hello, great news!

Can you point me to something where I can replicate your setup? I am mostly interested into seeing see how you use es-module-shims. So I can reproduce your setup on my machine and see what is happening.

[dmail]

I tried to make the code example available at https://github.com/lume/element#manipulating-and-composing-trees-of-elements working with es-module-shims and jsenv generating the import map file.

In the end I got an html page displayed without error but the counter does not increment. I have not tried to understand why at this point. Note: I have used html tagged template to avoid dependency on babel (https://github.com/dmail-fork/lume-test/blob/5a886a8524bb5a21f5307d9ad99689879f49bdc1/index.js#L2)

I made a GitHub repository with the files available at https://github.com/dmail-fork/lume-test.

To me you have some "missing" remappings because when you use an import like

import { foo } from "whatever/directory/file.js"

import map must contain an remapping as the one below

imports: {
  "whatever/": "./node_modules/whatever/"
}

And there is several places where you use this kind of import specifier like https://github.com/lume/element/blob/41e4db94b6cc86689b00a8d63e37f989378d1a0e/src/html.ts#L1.

I forced them to be generated using packagesManualOverrides. See https://github.com/dmail-fork/lume-test/blob/5a886a8524bb5a21f5307d9ad99689879f49bdc1/generate-import-map.js#L12-L23.

jsenv is reading package.json exports field. This field was documented in Node 13: https://nodejs.org/docs/latest-v13.x/api/esm.html#esm_subpath_exports.

Getting these remapping is one of the thing you need to get the correct importmap and exports field from your package.json can be used to do that. Let me know how it goes :)

[dmail]

For the record I also use these kind of import and have the corresponding field in the @jsenv/node-module-import-map/package.json.

https://github.com/jsenv/jsenv-node-module-import-map/blob/8908cf68c716cde3843882dce3d9eb33e68f2f20/package.json#L17-L23

[dmail]

I am working on https://github.com/jsenv/jsenv-node-module-import-map/pull/27 to improve this scenario:

The goal of this pull request is that:

• some import cannot be resolved -> script emit a warning to tell you which import cannot be resolved and suggest solutions.
• all import resolved -> scripts ends without warning

The pull request will also enable the ability to detect extensionless import and generate mappings for them.

[dmail]

Version 13.0.0 is out! This version uses the generated importmap against your codebase and emits warning each time an import cannot be resolved. It should help to find the import that will not be found by a browser without having to run the code.

[trusktr]

Wow, that's awesome you tried that lume/element attempt. I will inspect that repo and will circle back.

Sorry for the late replies. I'm slowly working on this. I have a new project in which I have been manually making my import map, one line at a time, as I encounter each dependency that is not recognized. After I get it working, I plan to circle back to this by by trying to auto-generate the map instead of the manual one, and seeing how it goes and how it compares.

jsenv is reading package.json exports field. This field was documented in Node 13: https://nodejs.org/docs/latest-v13.x/api/esm.html#esm_subpath_exports.

Getting these remapping is one of the thing you need to get the correct importmap and exports field from your package.json can be used to do that. Let me know how it goes :)

That is great! I'm using TypeScript which does not yet support this field, but they have committed to make it happen. I suppose I may need to use packagesManualOverrides for the time being.

 "exports": { 
   ".": { 
     "import": "./index.js", 
     "require": "./dist/commonjs/main.cjs" 
   }, 
   "./": "./" 
 },

Regarding that, isn't a missing exports field in package.json the equivalent of having exports: {"./": "./"} in package.json? Maybe that should be the default. From what I understood, adding an exports field allows re-mapping and hiding files, otherwise they are all visible by default in Node.js.

Version 13.0.0 is out!

Niice! :tada: I will try soon!

[dmail]

By the way exports: {"./": "./"} is deprecated and exports: {"./*": "./*"} should be used instead. Read more in https://github.com/jsenv/jsenv-node-module-import-map/issues/25.

isn't a missing exports field in package.json the equivalent of having exports: {"./": "./"} in package.json

I have just tested and I confirm you are right. @jsenv/node-module-import-map should generate a mapping to allow importing any file when "exports" field is not declared in the package.json 👍 . I have opened https://github.com/jsenv/jsenv-node-module-import-map/issues/35 to track this.

[dmail]

It should be fixed, feel free to comment if that's not the case.

[trusktr]

Thanks for all the work here! I will circle back again sometime soon.

In the end I got an html page displayed without error but the counter does not increment. I have not tried to understand why at this point.

Sidenote, a bit late, but the html template (html is from Solid.js) wasn't reactive because it needs a getter function. Instead of html`<div>${count()}</div>` it would need to be html`<div>${count}</div>` or html`<div>${() => count()}</div>`.

I've gotten everything working nicely as plain ESM since last circled here. I now also commit the JS outputs to the repo, so it is possible to simply view a Lume demo right from the repo:

https://raw.githack.com/lume/lume/develop/examples/fbx-model.html

If you view source, eventually you'll find the importmap is written by hand here:

https://raw.githack.com/lume/lume/develop/examples/importmap.js

It is not in the HTML directly, but at the top the <script src="template.js" ...> loads the top-level HTML template, which loads the importmap.

I'd like to circle back and see how importmap generation can help here.

The Lume installation docs currently show how to use JSPM online importmap generator to get started:

https://docs.lume.io/guide/install/

It'll be nice to add some details for the Local install method on how to generate an import map with @jsenv/importmap-node-module next!

[dmail]

Nice to read you again, I'll try to install lume locally and come back with suggestions, maybe a pr, to generate import map programmatically. At some point next week

[dmail]

About current documentation "local-buildless"

Could make it work, but had to update the documentation importmap

- "@lume/kiwi": "./node_modules/@lume/kiwi/es/kiwi.js",
+ "@lume/kiwi": "./node_modules/@lume/kiwi/dist/kiwi.js",

About documentating a programmatic way to generate importmap

In this section of the documentation I recommend to keep the pre-generated importmap you have and add an other paragraph to suggest using an importmap file + generate it programmatically (using @jsenv/importmap-node-module command below)

<script type="importmap" src="project.importmap"></script>

npx @jsenv/importmap-node-module ./project.importmap

But if you keep things as they are project.importmap is very big. There is 2 things you can do to have an importmap file with a size comparable to the one you generated manually:

1. Move dependency "@esm-bundle/chai" in lume/package.json from "dependencies" to "devDependencies". This packages pulls a lot of other packages and looks more like a dev dependency to me. It even looks like your repository does not use it at all so maybe remove "@esm-bundle/chai" entirely?
2. Add --remove-unused to the command to ensure mappings not used in source files are removed:

npx @jsenv/importmap-node-module ./src/project.importmap --entrypoint=./main.html --remove-unused

It will produce the following importmap:

{
  "imports": {
    "lume": "./node_modules/lume/dist/index.js"
  },
  "scopes": {
    "./node_modules/@lume/three-projected-material/": {
      "three/src/": "./node_modules/three/src/"
    },
    "./node_modules/element-behaviors/": {
      "@lume/custom-attributes/": "./node_modules/@lume/custom-attributes/",
      "solid-js/store": "./node_modules/solid-js/store/dist/store.js",
      "solid-js": "./node_modules/solid-js/dist/solid.js"
    },
    "./node_modules/@lume/autolayout/": {
      "@lume/kiwi": "./node_modules/@lume/kiwi/dist/kiwi.js"
    },
    "./node_modules/@lume/variable/": {
      "lowclass": "./node_modules/lowclass/dist/index.js",
      "solid-js": "./node_modules/solid-js/dist/solid.js"
    },
    "./node_modules/@lume/element/": {
      "@lume/variable": "./node_modules/@lume/variable/dist/index.js",
      "solid-js/web": "./node_modules/solid-js/web/dist/web.js"
    },
    "./node_modules/james-bond/": {
      "lowclass": "./node_modules/lowclass/dist/index.js"
    },
    "./node_modules/solid-js/": {
      "solid-js/web": "./node_modules/solid-js/web/dist/web.js",
      "solid-js": "./node_modules/solid-js/dist/solid.js"
    },
    "./node_modules/three/": {
      "three": "./node_modules/three/build/three.module.js"
    },
    "./node_modules/lume/": {
      "@lume/three-projected-material/": "./node_modules/@lume/three-projected-material/",
      "three/examples/jsm/": "./node_modules/three/examples/jsm/",
      "element-behaviors": "./node_modules/element-behaviors/dist/index.js",
      "@lume/autolayout": "./node_modules/@lume/autolayout/dist/AutoLayout.js",
      "@lume/eventful": "./node_modules/@lume/eventful/dist/index.js",
      "@lume/variable": "./node_modules/@lume/variable/dist/index.js",
      "@lume/element": "./node_modules/@lume/element/dist/index.js",
      "solid-js/html": "./node_modules/solid-js/html/dist/html.js",
      "solid-js/web": "./node_modules/solid-js/web/dist/web.js",
      "james-bond": "./node_modules/james-bond/dist/index.js",
      "three/src/": "./node_modules/three/src/",
      "lowclass": "./node_modules/lowclass/dist/index.js",
      "solid-js": "./node_modules/solid-js/dist/solid.js",
      "regexr": "./node_modules/regexr/dist/index.js",
      "three": "./node_modules/three/build/three.module.js"
    }
  }
}