Sugar icon indicating copy to clipboard operation
Sugar copied to clipboard

Sugar throwing a runtime exception in Angular v6 application.

Open phixMe opened this issue 6 years ago • 9 comments

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.

phixMe avatar May 05 '18 02:05 phixMe

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?

andrewplummer avatar May 05 '18 03:05 andrewplummer

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; 

phixMe avatar May 05 '18 05:05 phixMe

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.

YurkaninRyan avatar May 15 '18 15:05 YurkaninRyan

I doubt that strict mode makes this object undefined.

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

window['global'] = window;

tasuku-s avatar Jun 20 '18 12:06 tasuku-s

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

vendethiel avatar Jun 20 '18 12:06 vendethiel

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

gaiottino avatar Jun 22 '18 12:06 gaiottino

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

tasuku-s avatar Jun 24 '18 05:06 tasuku-s

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.

andrewplummer avatar Jul 27 '18 08:07 andrewplummer

Is https://github.com/andrewplummer/Sugar/commit/0cd73d1fc83e16a7b51e02c5b19b9db18d7ac180 intended to be the fix for this issue?

jikamens avatar Aug 04 '19 15:08 jikamens