Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ES6 Shim fails in Bundle #392

Open
SebastianStehle opened this issue Dec 21, 2015 · 50 comments
Open

ES6 Shim fails in Bundle #392

SebastianStehle opened this issue Dec 21, 2015 · 50 comments

Comments

@SebastianStehle
Copy link

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.

@ljharb
Copy link
Collaborator

ljharb commented Dec 21, 2015

@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.

@SebastianStehle
Copy link
Author

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.

@ljharb
Copy link
Collaborator

ljharb commented Dec 21, 2015

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

@SebastianStehle
Copy link
Author

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

@ljharb
Copy link
Collaborator

ljharb commented Dec 22, 2015

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.

@stroborobo
Copy link

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.

@ljharb
Copy link
Collaborator

ljharb commented Feb 10, 2016

@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.

@stroborobo
Copy link

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! :)

@ljharb
Copy link
Collaborator

ljharb commented Feb 11, 2016

@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.

@stroborobo
Copy link

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/

@ljharb
Copy link
Collaborator

ljharb commented Feb 11, 2016

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

@ljharb
Copy link
Collaborator

ljharb commented Feb 11, 2016

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.

@stroborobo
Copy link

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!

@SebastianStehle
Copy link
Author

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

Uncaught TypeError: Cannot redefine property: Reflect

@ljharb
Copy link
Collaborator

ljharb commented Feb 23, 2016

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

@HafizAhmedMoon
Copy link

@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>

@ljharb
Copy link
Collaborator

ljharb commented Feb 26, 2016

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

@HafizAhmedMoon
Copy link

But, It's working...

@ljharb
Copy link
Collaborator

ljharb commented Feb 26, 2016

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.

@HafizAhmedMoon
Copy link

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.

@ljharb
Copy link
Collaborator

ljharb commented Feb 26, 2016

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

@HafizAhmedMoon
Copy link

@ljharb you are right, but if you want to

es6-shim to be robust against it

you have to do like this :)

@ljharb
Copy link
Collaborator

ljharb commented Feb 26, 2016

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
Copy link
Collaborator

ljharb commented Feb 28, 2016

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?

@HafizAhmedMoon
Copy link

This error occurs after merging files into one.

@ljharb
Copy link
Collaborator

ljharb commented Feb 28, 2016

@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.

@HafizAhmedMoon
Copy link

@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.

@ljharb
Copy link
Collaborator

ljharb commented Mar 1, 2016

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

@SebastianStehle
Copy link
Author

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

@ljharb
Copy link
Collaborator

ljharb commented Mar 2, 2016

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.

@cezarykluczynski
Copy link

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.

@HafizAhmedMoon
Copy link

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

@jdelobel
Copy link

jdelobel commented Apr 5, 2016

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

@ljharb
Copy link
Collaborator

ljharb commented Apr 5, 2016

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

@jdelobel
Copy link

jdelobel commented Apr 6, 2016

@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)

@DennisDel
Copy link

DennisDel commented Apr 27, 2016

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

@ljharb
Copy link
Collaborator

ljharb commented Apr 27, 2016

@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?

@DennisDel
Copy link

@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.

@ljharb
Copy link
Collaborator

ljharb commented Apr 27, 2016

@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?

@DennisDel
Copy link

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

@ljharb
Copy link
Collaborator

ljharb commented Apr 27, 2016

@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)

@DennisDel
Copy link

Will do. Thank you.

@NoMan2000
Copy link

NoMan2000 commented May 18, 2016

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.

@ljharb
Copy link
Collaborator

ljharb commented May 18, 2016

@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", {)

@DennisDel
Copy link

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
Copy link

DennisDel commented Jun 6, 2016

@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
Copy link

DennisDel commented Jun 6, 2016

This is my current problem. It is on IE11
title

@ljharb
Copy link
Collaborator

ljharb commented Jun 6, 2016

@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)

@ps37
Copy link

ps37 commented Jul 6, 2017

@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?

@ljharb
Copy link
Collaborator

ljharb commented Jul 7, 2017

@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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants