standard-things / esm

Tomorrow's ECMAScript modules today!
Other
5.26k stars 146 forks source link

`esm` incorrectly proxies `Reflect` global, breaking `reflect-metadata`-like polyfills #809

Open Swatinem opened 5 years ago

Swatinem commented 5 years ago

When using import to import this polyfill, or when it is done transitively via a dependency that I import, the "reflect-metadata" polyfill does not work correctly.

> npm i esm reflect-metadata
+ reflect-metadata@0.1.13
+ esm@3.2.25

> node -r esm -e "require('reflect-metadata');console.log(Reflect.metadata)"
[Function: metadata]
> node -r esm -e "import 'reflect-metadata';console.log(Reflect.metadata)"
undefined

> echo "require('reflect-metadata')" > dep.js
> node -r esm -e "require('./dep.js');console.log(Reflect.metadata)"
[Function: metadata]
> node -r esm -e "import './dep.js';console.log(Reflect.metadata)"
undefined
Swatinem commented 5 years ago

hm, looks like there is the same problem with @abraham/reflection which is an alternative to reflect-metadata and basically does a Object.assign(Reflect, Reflection); on the global Reflect object.

So I guess esm messes with global in some way that breaks these kind of polyfills?

> node -r esm -e "require('@abraham/reflection');console.log(Reflect.metadata)"
[Function: metadata]
> node -r esm -e "import '@abraham/reflection';console.log(Reflect.metadata)"
undefined
Swatinem commented 5 years ago

Hm, seems like esm defines its own shim of Reflect and thus messes with anything that tries to use that.

> echo "export default { Array, Error, Reflect };" > dep.js          
> node -r esm -e "const dep = require('./dep.js').default;console.log(dep.Array === Array, dep.Error === Error, dep.Reflect === Reflect)"
true true true
> node -r esm -e "import dep from './dep.js';console.log(dep.Array === Array, dep.Error === Error, dep.Reflect === Reflect)" 
true true false
Adamfsk commented 5 years ago

Getting similar issues while trying to use dependency injection libraries with esm. Makes them unusable.

I suspect the issue is coming from builtin/reflect.js which is doing some wrapping of Reflect methods.

resynth1943 commented 4 years ago

Wow. Thank you. I've been experiencing this too.

Take a file, we'll call it a.js.

import 'reflect-metadata';
console.log(typeof Reflect.metadata);

Now then, let's run it with node --experimental-modules a.js. It logs function. When I run it with node -r esm a.js however, undefined is logged. I've been battling this for hours.

/cc @sindresorhus This is still an issue.

resynth1943 commented 4 years ago

Oh, yeah. I was looking at builtin/reflect.js too. That seems to be causing this, @Adamfsk. Hopefully this issue will be mitigated, and I can actually run my app. Until then, TypeORM is screaming at me before my DI library gets a chance to. :heart:

Adamfsk commented 4 years ago

@resynth1943 Take a look at tsoa if TypeORM is proving to be a pain!

brokenmass commented 4 years ago

@jdalton : can you please provide an update ? this is a deal breaker for me (and i guess for many others)

resynth1943 commented 4 years ago

Oh my, this still hasn't been fixed? My solution was not using this package.