standard-things / esm

Tomorrow's ECMAScript modules today!
Other
5.27k stars 147 forks source link

esm error: Identifier '__global__' has already been declared #816

Open pvdlg opened 5 years ago

pvdlg commented 5 years ago

This is the same issue as #671. This was fixed by 9c2ecdf21b696c02068a0ed34e00a361ada263f7. However it seems the fix was reverted in 876f043955a521340a61327636f9265f353e49ed so the problem happens again.

Any reason why it was reverted?

jdalton commented 5 years ago

Any reason why it was reverted?

@pvdlg Lack of a proper unit test. Would you be up for creating one?

pvdlg commented 5 years ago

I can look at it. But some guidance would be welcomed!

bobjunga commented 4 years ago

I have this error also in an Atom package (i saw your issue in eslint-plugin-ava #210 too).

I wrote an atom plugin and an npm library package that the atom package uses. I first converted the library to esm and it worked. Then I converted the atom package and it gets this error. I suspect that its related to the esm loader instance of the atom package reaching the library which uses esm and conflicting somehow.

I am at the point of wondering if I am just doing an unsupported or incorrect configuration or if its an actual bug that I should track down.

I am runing a recent cloned atom and getting esm 3.2.25 from npm.

I am not using rollup in the library -- just the typical index.js/main.js cjs pattern.

(btw, I am pretty new to node dev ecosystem, but I am an experience systems developer)

--BobG

bobjunga commented 4 years ago

FYI, in my library's index.js, I put a guard 'if (!_._global__) require = require("esm")(module)' and it works.

It seems that at the npm package boundary, esm does not handle files that it expects to be cjs but are cjs compatible by virtual of using esm thus creating nested esm instances.

I suspect that this is not typically a problem because a library npm package typically will use rollup to apply esm to produce an actual cjs compatible file that the calling package's loader, what ever it is, will load without having to invoke a second esm.

I wonder if it would be worthwhile for you to put a similar guard in esm itself so that it would internally handle crossing these boundaries.

Or, if you want to encourage people to use esm only in the build cycle, you could make the guard throw a clear exception so people know they are in an unsupported situation of creating a second esm loader.

(edit) Hmm, maybe the issue is that the ems instance of the Atom package should have recognized the "module" attribute in the library's package.json and loaded it instead of the "main" attribute. I will check to make sure I have that declared correctly (sorry I did nto think of that before I wrote this comment...)

--BobG

bobjunga commented 4 years ago

Update,

I am putting the if (!_._global__) guard in each of my packages entry point files for the time being as an ugly work-a-round.

My best understanding of it so far is that esm has a bug where it is not recognizing pkg.module over pkg.main in modules folders that it loads so its trying to replace the current require function which is already esm with another instance of esm and failing b/c _._global__ is const.

I tried tracing the esm code but eventually I gave up. (BTW, there is no shame in including a few conceptual comments:) I wanted to see when it made the decision to load index.js (referenced in pkg.main) and not main.js (referenced in pkg.module). I could not find that and I kind of suspect that its deferring to the larger environment's module lookup which would always prefer pkg.main over pkg.module.

--BobG

bobjunga commented 4 years ago

update: I now find that one Atom package works fine with the work-a-round of checking to see if global already exists. But if I have two packages that use that pattern, the second one that Atom loads fails.For the second package, global exists, but require is not the esm loader. I infer that because the js module fails with a syntax error on the first import statement.

Atom is an electron application with a main process and a render process for each window it opens. I assume that packages are loaded in each render process separately. The atom code uses require to load the package.. It seems that global constant that esm creates is in scope across multiple package loads.but the require variable is reset for each package load.

This is the new work-a-round that allows me to load multiple atom packages that use ems. In the index.js (package.json:main == 'index.js', package.json:module == 'myEsmFileEntryPoint' )

if (typeof global.__bgEsmModule__ == 'undefined')
    global.__bgEsmModule__ = require("esm")
require = global.__bgEsmModule__(module/* , options */);
module.exports = require(require('./package.json').module).default

--BobG