es6-shim icon indicating copy to clipboard operation
es6-shim copied to clipboard

ES6 Shim fails in Bundle

Open SebastianStehle opened this issue 9 years ago • 50 comments

Hi, I created a bundle with es6-shim as first file.

Unfortunatly my app (http://mobile.green-parrot.net/) fails, because of this line of code.

3215: if (!ES.IsCallable(globals.Reflect[key])) { }

The reason is that globals.Reflect is undefined from beginning.

SebastianStehle avatar Dec 21 '15 09:12 SebastianStehle

@SebastianStehle i see you closed this; were you able to resolve the issue? https://github.com/paulmillr/es6-shim/blob/master/es6-shim.js#L263-L266 defines globals.Reflect in the beginning.

ljharb avatar Dec 21 '15 17:12 ljharb

Partially, I referenced the es6-shim from a cdn and it works. I saw the lines of code you mentioned and I debugged through it, but it was very strange. Because globals.Reflect exists and was undefined your code didnt create a valid reference.

SebastianStehle avatar Dec 21 '15 20:12 SebastianStehle

Are you saying that the last published version works, but master fails?

ljharb avatar Dec 21 '15 22:12 ljharb

No, it doesnt work for me when I bundle it. When I reference es6-shim.js as standalone it works.

SebastianStehle avatar Dec 22 '15 09:12 SebastianStehle

es6-shim is required to be run such that this is the global object, and should be the first code executed on your page (after the es5-shim). It should only be the case that it doesn't work if your bundling pipeline isn't packaging it correctly.

ljharb avatar Dec 22 '15 17:12 ljharb

I can reproduce this, cating multiple files into one where es6-shim is the first one breaks. Shouldn't that be the same as including those files in <head> in the same order? No fancy bundling.

stroborobo avatar Feb 10 '16 14:02 stroborobo

@stroborobo if you are naively concatenating multiple files together without even properly separating them with semicolons, a lot of things will break, due to ASI rules. Simple cating has never been a correct or safe approach, and at this point is a very bad web dev practice. In 2016, bundling and/or a build process is simply required.

ljharb avatar Feb 10 '16 16:02 ljharb

Hey Jordan, I'm very much aware of that :) I wasn't talking about syntax errors or similar. But you're right of course, I just wanted something quick up and running, so I could dabble with the things I was interested in right now. (Eventually use SystemJS, but well, one step at a time, still new to this modern Javascript dependency management.)

But to have that written down: it's Angular2's polyfills that declared Reflect in global space.

Thanks anyway and have a great day! :)

stroborobo avatar Feb 11 '16 08:02 stroborobo

@stroborobo Angular2 is big enough that I'd be willing to make a small patch to handle that case. However, https://github.com/paulmillr/es6-shim/blob/master/es6-shim.js#L264-L267 should be working with it. If you can make me a jsfiddle that reproduces the problem I'll be happy to look into it further.

ljharb avatar Feb 11 '16 08:02 ljharb

Ok, the exact problem is this: angular2-polyfills.js has a var Reflect; in global space, which results an undefined window.Reflect. Your check !globals.Reflect is true but in defineProperty the check is actually name in object which is true, since it's declared but not defined to something evaluating to true.

If you really need a jsfiddle I'd make one, but I guess that's a fairly simple problem.

EDIT: whatever. http://tmp.kbct.de/es6-shim/

stroborobo avatar Feb 11 '16 08:02 stroborobo

Aha, thank you. Seems simple enough, I'll put up a fix.

ljharb avatar Feb 11 '16 08:02 ljharb

also, it would be good to file a bug report on angular for that behavior - they shouldn't be a) creating globals nor b) using names that are part of the language.

ljharb avatar Feb 11 '16 08:02 ljharb

I'll have a look into their source code, maybe it's just the Typescript compiler outputting things that weren't intended. Thanks for adding this workaround!

stroborobo avatar Feb 11 '16 09:02 stroborobo

Hi, thank you for your fix, I finally tried it again, but I have another exception now:

Uncaught TypeError: Cannot redefine property: Reflect

SebastianStehle avatar Feb 23 '16 10:02 SebastianStehle

@SebastianStehle any chance you could make me a jsfiddle that reproduces the issue?

ljharb avatar Feb 23 '16 18:02 ljharb

@SebastianStehle keep the angular2-polyfills at above from es6-shim

  <script src="node_modules/angular2/bundles/angular2-polyfills.js"></script>
  <script src="node_modules/es6-shim/es6-shim.js"></script>
  <script src="node_modules/systemjs/dist/system-polyfills.js"></script>

  <script src="node_modules/systemjs/dist/system.src.js"></script>
  <script src="node_modules/rxjs/bundles/Rx.js"></script>
  <script src="node_modules/angular2/bundles/angular2.dev.js"></script>

HafizAhmedMoon avatar Feb 26 '16 01:02 HafizAhmedMoon

That's not a good solution, es5-shim and es6-shim should always be first :-)

ljharb avatar Feb 26 '16 01:02 ljharb

But, It's working...

HafizAhmedMoon avatar Feb 26 '16 01:02 HafizAhmedMoon

lots of things "work" :-) if there's a bug in angular 2's polyfills, i'd prefer es6-shim to be robust against it if possible, and better, i'd prefer angular 2 fix their broken code.

ljharb avatar Feb 26 '16 01:02 ljharb

You have to change this from

var defineProperty = function (object, name, value, force) {
    if (!force && name in object) { return; }
    if (supportsDescriptors) {
      Object.defineProperty(object, name, {
        configurable: true,
        enumerable: false,
        writable: true,
        value: value
      });
    } else {
      object[name] = value;
    }
  };

to

var defineProperty = function (object, name, value, force) {
    if (!force && name in object) { return; }
    if (supportsDescriptors && Object.isExtensible(object[name])) { // because Reflect in window but undefined and non-extensible
      Object.defineProperty(object, name, {
        configurable: true,
        enumerable: false,
        writable: true,
        value: value
      });
    } else {
      object[name] = value;
    }
  };

it's working.

HafizAhmedMoon avatar Feb 26 '16 02:02 HafizAhmedMoon

@hafizahmedattari is that because angular 2's polyfill makes it non-extensible (which is a violation of the spec)?

ljharb avatar Feb 26 '16 07:02 ljharb

@ljharb you are right, but if you want to

es6-shim to be robust against it

you have to do like this :)

HafizAhmedMoon avatar Feb 26 '16 11:02 HafizAhmedMoon

Not necessarily - even if it's made non-extensible I can still just delete it, and copy its properties over to a new object.

ljharb avatar Feb 26 '16 16:02 ljharb

When I use https://cdnjs.cloudflare.com/ajax/libs/angular.js/2.0.0-beta.7/angular2-polyfills.js - I can't reproduce this whether it's included before or after es6-shim. Is there a specific browser this fails in?

ljharb avatar Feb 28 '16 18:02 ljharb

This error occurs after merging files into one.

HafizAhmedMoon avatar Feb 28 '16 20:02 HafizAhmedMoon

@hafizahmedattari that doesn't make any sense - short of ASI issues (the shims use a UMD so that doesn't apply) and strict mode, two script tags are identical to two concatenated files from a runtime standpoint.

ljharb avatar Feb 28 '16 22:02 ljharb

@ljharb I had debugged the result of globals.Reflect at Line:265 in both situations(separate files and bundle file). The result was shock, with separate files the value of globals.Reflect is object and with bundle file the value of globals.Reflect is undefined. That's why it goes to redefining Reflect, which is already defined in globals with undefined value.

HafizAhmedMoon avatar Mar 01 '16 14:03 HafizAhmedMoon

If you could provide a jsfiddle (or similar) with the bundle file that would be very helpful

ljharb avatar Mar 01 '16 19:03 ljharb

I just reopened the issue, because it is not solved yet. I hope it is okay.

SebastianStehle avatar Mar 02 '16 12:03 SebastianStehle

Absolutely :-)

However, I will say that while I'm very curious to figure it out, the implication is that there's a flaw in your bundling process, since the files work separately - and any time bundling changes behavior it's usually a bundling bug.

ljharb avatar Mar 02 '16 16:03 ljharb

I was doing Angular2 hello world and stumbled upon this problem too, when trying to concatenate all required dependencies into one files. My solution was to wrap content of every file into self-executing anonymous function.

cezarykluczynski avatar Mar 20 '16 12:03 cezarykluczynski

Issue resolved in updated angular2 lib, I tried in [email protected], working without any problem.

HafizAhmedMoon avatar Mar 30 '16 20:03 HafizAhmedMoon

Same problem with [email protected] for me (during minification process).

jdelobel avatar Apr 05 '16 11:04 jdelobel

@jdelobel can you make me a jsfiddle that reproduces it with the latest version of the shim?

ljharb avatar Apr 05 '16 15:04 ljharb

@ljharb You can clone https://github.com/jdelobel/angular2-tour-of-heroes/tree/develop. Then:

  • npm i
  • gulp dist
  • npm start and you will see the error.

Browser: Chromium Version 47.0.2526.106 Built on Ubuntu 14.04, running on LinuxMint 17.3 (64-bit)

jdelobel avatar Apr 06 '16 09:04 jdelobel

Same problem with latest version. Uncaught ReferenceError: a is not defined. Please see the attachment for problem line. title

DennisDel avatar Apr 27 '16 03:04 DennisDel

@DennisDel This looks like a bad minification - it's changing "Map" to "a", but "a" should only be available inside the constructor. Is that the minified build, or if not, what tool are you using to produce that?

ljharb avatar Apr 27 '16 04:04 ljharb

@ljharb I am using VS 2015 and used Google Chrome pretty print just to show you what the problem is a bit more specifically. I am not using Angularjs 2.0 btw.

DennisDel avatar Apr 27 '16 04:04 DennisDel

@DennisDel VS 2015 is an editor - what i meant was, are you using the minified build included in this repo, or is your app doing the minification itself?

ljharb avatar Apr 27 '16 04:04 ljharb

@ljharb I tried minified and normal from this repo. they both have the same problem when I build them in release mode.

DennisDel avatar Apr 27 '16 04:04 DennisDel

@DennisDel thanks - i think you have a separate issue. would you file a new one with this information, as well as what build process you're using? (if that's something built into VS 2015, mention that too, it may have a bug)

ljharb avatar Apr 27 '16 05:04 ljharb

Will do. Thank you.

DennisDel avatar Apr 27 '16 05:04 DennisDel

I also encountered this. When using the ES6 Sham, (but not the ES5 Shim/Sham and ES6 Shim), whenever I minified the files using Uglifier, Safari started throwing errors.

This is the part it finds objectionable, TypeError: Attempting to change configurable attribute of unconfigurable property.

             }, set instanceof Object ? setPrototypeOf = createAndCopy : (set.__proto__ = objProto, setPrototypeOf = set instanceof Object ? function(origin, proto) {
                return origin.__proto__ = proto, origin
            } : function(origin, proto) {
                return getPrototypeOf(origin) ? (origin.__proto__ = proto, origin) : createAndCopy(origin, proto)
            })
        }
        Object.setPrototypeOf = setPrototypeOf
    }
}(), supportsDescriptors && "foo" !== function() {}.name && Object.defineProperty(Function.prototype, "name", {

It's the Object.defineProperty on "foo" !== function that it seems to take umbrage with.

NoMan2000 avatar May 18 '16 20:05 NoMan2000

@NoMan2000 if you're using uglifier, by default it mangles function names, which is why i provide a pre-uglified es6-shim.min.js. To unbreak you, you can use the uglifier option to not do that. (Namely, the line should be }(), supportsDescriptors && "foo" !== function foo() {}.name && Object.defineProperty(Function.prototype, "name", {)

ljharb avatar May 18 '16 23:05 ljharb

Hey all, if you have similar problems, make sure you always use the minifed version and DO NOT use JsMinify to do it. I tried both normal and minified version and they always cause issues with JsMinify.

DennisDel avatar Jun 03 '16 03:06 DennisDel

@ljharb Do I need any other file to include in my bundle or es6-shim.min.js alone (and of course the def file) will suffice ?

DennisDel avatar Jun 06 '16 00:06 DennisDel

This is my current problem. It is on IE11 title

DennisDel avatar Jun 06 '16 01:06 DennisDel

@DennisDel this RangeError is wrapped by a function that does try/catch - I'm sure you'll find it called a in that screenshot, somewhere in the code - so I'm not sure how that error isn't being caught. Can you make a jsfiddle that reproduces it?

(es6-shim is recommended to follow es5-shim, since every browser violates ES5 in some way, but it should work just fine without it too)

ljharb avatar Jun 06 '16 05:06 ljharb

@ljharb @HafizAhmedMoon I am still getting the following error only on IE11. SCRIPT5078: Cannot redefine non-configurable property 'Reflect'

I am bundling the es6-shim.min.js before all my sources and I am using the latest version of Angular. Is the issue resolved?

ps37 avatar Jul 06 '17 18:07 ps37

@ps37 Can you try including es6-shim in a separate bundle, before Angular, and make sure all script tags are either omitting or including the defer attributes, and all omitting async? (ie, so all the tags execute in order)

ljharb avatar Jul 07 '17 04:07 ljharb