Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

repro for issue #366 #369

Closed
dnalborczyk opened this issue Apr 19, 2018 · 7 comments
Closed

repro for issue #366 #369

dnalborczyk opened this issue Apr 19, 2018 · 7 comments
Labels

Comments

@dnalborczyk
Copy link
Contributor

dnalborczyk commented Apr 19, 2018

@jdalton just looked at #366 again.

repro:
https://github.com/dnalborczyk/esm-issue-366

versions are locked down.

cd app1
npm i  # or better npm ci
npm start

it seems the existence of a package.json file has an effect on the outcome of the import, which I would have not expected either. I thought package.json would only be valued if one wants to import a package without a direct path but with a "main" entry specified.

some observations, which I'm not sure they are intended:

  • it does not matter if app1 has a higher or lower version of esm installed than app2 (fails nonetheless)
  • cache folder is being created for App2 (even if node_modules doesn't exists prior)
@jdalton
Copy link
Member

jdalton commented Apr 19, 2018

cache folder is being created for App2 (even if node_modules doesn't exists prior)

That's fine because it's telling App2 that its cache (when it does run) is dirty.

it does not matter if app1 has a higher or lower version of esm installed than app2 (fails nonetheless)

I cannot repro that. If I make App2 pin at 3.0.19 then the app works. This looks like the same root issue which is you're explicitly telling esm not to work there by pinning one app to an older version. If I change App2 from 3.0.16 to ^3.0.16 then App1 reaching into App2 will also work.

If App2 used require("esm")(module) to load and export its package then there is no issue either (regardless of App1 esm version).

I can handle this in a couple ways.

  • I can improve the error message to state the expected version and notify if the expected version is pinned (no leading ^, ~, >, or <).

  • I can remove the gate entirely. The esm gate was added way back in @std/esm when it was on the global hook by default and so it was super important to not step on other packages. However, since this is the local app scenario I could just say yolo to it and the leading loader wins. This means if the source you're trying to load expects a newer version (with a feature not supported) things will just not work as expected.

@dnalborczyk
Copy link
Contributor Author

dnalborczyk commented Apr 19, 2018

it does not matter if app1 has a higher or lower version of esm installed than app2 (fails nonetheless)

I cannot repro that.

sorry, I literally meant higher or lower, not same (should have emphasized)

what I'm lacking is the misunderstanding that the mere presence of a package.json file in another folder has an effect on how the file is being loaded with esm. though I have no knowledge on what node itself does (without using esm). meaning, if I "cjs" require from a folder with a package.json, is node interested in anything from package.json (besides the "main" field, if specified)?

@dnalborczyk
Copy link
Contributor Author

dnalborczyk commented Apr 19, 2018

just re-reading your comments. is the reason esm is looking for a package.json dependency in the folder of the file being loaded to make sure that the features are supported by the esm version the root app is using?

btw, if esm is being specified as a dev-dependency in package.json (which I'm doing for e.g. loading an ESM version of webpack.config) in app2 then there's no error.

@dnalborczyk
Copy link
Contributor Author

maybe my misunderstanding is also related to the intended usage for module packages.

e.g lodash/lodash#3745 I suppose you'd make the lodash entry point for cjs consumers with esm. if someone wants to use the ESM modules of lodash v5, I suppose they would need to use esm -- for the time being -- unless you're going with *.mjs.

assuming someone would be referencing:

import { pick } from "lodash"
// or
import pick from "lodash/pick"

why would esm [loaded from the root app] care about the esm module in dependencies which is only loaded (and meant) for the cjs consumers to create an entry point in lodash?

@jdalton jdalton closed this as completed Apr 19, 2018
@jdalton
Copy link
Member

jdalton commented Apr 19, 2018

You're a bit all over the place with this so I'll just brain dump how it is. The loader is mimicking how npm deps work. By pinning to a specific version you are stating only run that version. That said I can gate to the classic semver ^ for local runs and as well as improve the error for the next major bump (v4). The esm loader already errors when it doesn't recognize a provided option so I don't have to worry about mystery fails on that front (though I could improve that message as well).

@dnalborczyk
Copy link
Contributor Author

dnalborczyk commented Apr 19, 2018

btw, just to make sure, I'm not arguing for anything, just trying to understand.

yeah, I know how npm does semver (to a certain degree). but that's not so much the issue. I also don't know what action esm should take on matters of ^, ~, or *. I have no opinion on that. I haven't thought about it, since I don't understand why the version information is being taken into consideration in the first place 😃

what it comes down to is:
when I'm referencing a file from a folder, which happens to have a package.json file, with an entry for a dependency on esm (but nothing inside that folder has a reference to esm, as well as there's no node_modules folder inside), why is the esm version being taken into consideration for the app referencing a file from that folder?

Maybe it's some inner workings of esm I'm not aware of.

just added, maybe that's the misunderstanding: with esm, I was always talking about the loader, not the language feature. maybe I should have been more explicit.

@jdalton
Copy link
Member

jdalton commented Apr 19, 2018

v3.0.20 is released 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

2 participants