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

Sugar throwing a runtime exception in Angular v6 application. #632

Open
phixMe opened this issue May 5, 2018 · 9 comments
Open

Sugar throwing a runtime exception in Angular v6 application. #632

phixMe opened this issue May 5, 2018 · 9 comments
Labels
Milestone

Comments

@phixMe
Copy link

phixMe commented May 5, 2018

I have used the following import syntax for the Sugar date module through Angular v5.2.10 and CLI v1.7.4 with success. Upon upgrading the version of Angular and its companions I observe the following issue reported by Sugar v2.0.4.

import * as Sugar from 'sugar/date';

image

I have also tried the following without success...

import {Sugar} from 'sugar/date';

I understand that this may be an Angular related issue, but I was curious if others have observed this upon upgrading their Angular applications.

@andrewplummer
Copy link
Owner

Thanks for this! I'll have a look into it too but for a workaround can you get in there and try to figure out which global it's trying to reference?

@phixMe
Copy link
Author

phixMe commented May 5, 2018

I'll try to help out the best I can without being too familiar with the internals of the library.

It looks like this error occurs during the mapping of the NATIVE_NAMES constant specifically when name === 'Object'

forEachProperty(NATIVE_NAMES.split(' '), function(name) {
  createNamespace(name);
}); 

image

// seems to be undefined
globalContext["Object"].prototype; 

@YurkaninRyan
Copy link

YurkaninRyan commented May 15, 2018

When you log globalContext what do you get @phixMe?

I recently was having this problem in firefox content scripts. It looks like globalContext defaults to this, but in firefox this is not the window. This may be related.

@tasuku-s
Copy link

tasuku-s commented Jun 20, 2018

I doubt that strict mode makes this object undefined.

I fixed it by putting this workaround into polyfills.ts.

window['global'] = window;

@vendethiel
Copy link

I doubt that strict mode makes this object undefined.

That's one of the features of strict mode, though: https://coderwall.com/p/jes4dw/strict-mode-this-keyword-in-javascript

@gaiottino
Copy link

We just ran into this issue as well. @tasuku-s does your workaround have any consequences in terms of AOT etc?

@tasuku-s
Copy link

@gaiottino
I have no idea to answer this question.
But, it works with AOT build mode in my application.

I wonder why strict mode makes this undefined only under angular6 environment.
It seems like global issue, if sugar.js apply this to globalContext under strict mode.
In actual, sugar.js bind this without strict mode.
https://github.com/andrewplummer/Sugar/blob/master/dist/sugar.js#L13473

So maybe webpack or typescript wrap around with strict mode.
There is some option to solve this issue.

@andrewplummer
Copy link
Owner

Hi, sorry for the delay on this. I had a look into this issue.

It seems that there are a couple of factors here. First it seems that @tasuku-s is right that there is a wrapper with strict mode going on that makes the assumed this become not the global context. However this seems to be standard webpack behavior. However in addition there's something wonky going on with Angular's webpack environment that doesn't define a global keyword. This causes a fallback to standard browser environment, but of course webpack isn't a standard browser environment, so you are essentially running code in some kind of hybrid environment which is going to make edge cases come up. Standard webpack doesn't display this behavior. I'd be surprised if other libs haven't been hit by this as well.

The fix here is I think for Sugar to stop making assumptions about the environment it's being run in and perform more robust checks for both the global context as well as exports . Essentially, no more this, just get global or window explicitly.

@jikamens
Copy link
Contributor

jikamens commented Aug 4, 2019

Is 0cd73d1 intended to be the fix for this issue?

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

No branches or pull requests

7 participants