Skip to content

Become resilient to lack of global variable #401

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

Closed
Andarist opened this issue Aug 9, 2017 · 20 comments
Closed

Become resilient to lack of global variable #401

Andarist opened this issue Aug 9, 2017 · 20 comments

Comments

@Andarist
Copy link

Andarist commented Aug 9, 2017

Huge amount of users are compiling the lib from the source code when using bundlers like webpack, rollup and probably others.

Having a global variable is not guaranteed! I.e. with webpack3 we can specify node: false option to stop any node specific polyfills being added to the bundle - this makes global undefined and thus this lib stops working.

Imho we shouldnt rely on having it because it requires bundlers / users to provide it.

I'm committed to making a PR fixing this, but first I would like to establish if its something you would like to follow and how this should be accomplished - i.e. by wrapping detecting what global is in a module and importing it whenever we need it, or maybe some simple checks against undefined would suffice - but that needs more investigation later.

@brycekahle
Copy link
Contributor

global is aliased to window in browserify. It should be trivial to do something similar with other bundlers.

@Andarist
Copy link
Author

Andarist commented Aug 9, 2017

Sure it aint really hard, but now we have a situation when shipped code is not usable on its own, which I consider a bad thing.

@brycekahle
Copy link
Contributor

The code as written works without modification in Node.js. It is 100% required to use some sort of bundler to use in the browser (if you don't want to use the pre-built versions available in repo and on several CDNs).

What use case are you working on? This library needs references to window in a browser. It would theoretically be possible to do without in Node.js, but would result in duplication of some of the currently shared code.

@Andarist
Copy link
Author

Well, I've just assumed that the library targets to be isomorphic and is doing so by assuming global is provided by a bundler. I don't really have any use case here, just had to provide global manually in my bundle, because of this and thought that it could be avoided in the future for others, so I've filed the issue while also offering to help with the change if we agree if and how it should be done.

@lucidlemon
Copy link

Any update on this? It's really a pain in the ass to handle these errors.

@Andarist
Copy link
Author

Andarist commented Sep 1, 2017

Imho the best option would be to just create a global module which would export appropriate global variable based on a simple check and import this in every module which needs a reference to it instead of reaching to a global namespace.

Bryce seemed unconvienced when ive raised this issue though. Maybe could be reconsidered? Or closed as not wanted.

@filipesilva
Copy link

Heya, I'm also looking into this case in the context of Angular CLI (angular/angular-cli#5804).

We use webpack-dev-server as a development server, which in turn uses sockjs-client. Due to sockjs-client needing global to be present, webpack has to include at least the node.global (https://webpack.js.org/configuration/node/#node).

That's ok insofar as getting the dev server to run. But for browser apps it is incorrect to have global present. If a browser app looks for global, it should not be there because it is not a node environment.

There is a concrete case where this is problematic right now: mapbox/mapbox-gl-js#4359 (which is at the root of angular/angular-cli#5804). The comment there explains the problem pretty well, but the TLDR is that uglifying code that relies on webpack wrapping breaks with webworkify.

It is true that there are a bunch of ways the symptom can be fixed in packages that use sockjs-client (and even using the prebuilt js files). But it looks to me like the lines below are trying to figure out a fallback to when crypto isn't available:

if (global.crypto && global.crypto.getRandomValues) {
module.exports.randomBytes = function(length) {
var bytes = new Uint8Array(length);
global.crypto.getRandomValues(bytes);
return bytes;
};
} else {
module.exports.randomBytes = function(length) {
var bytes = new Array(length);
for (var i = 0; i < length; i++) {
bytes[i] = Math.floor(Math.random() * 256);
}
return bytes;
};
}

Since sockjs-client seems to be the browser client, I think it's reasonable for it to have a browser safe source code. This is of course not truly feasible since the source uses require() as well. But that doesn't mean that it shouldn't be safe in it's usage of global.

@FrozenDroid
Copy link

This is also a problem with Angular 6. Are we getting this fixed?

@aceleghin
Copy link

I have the same problem with angular 6, any fix/ workaround?

@drenda
Copy link

drenda commented May 29, 2018

Same problema with Angular6. Any advice? Thanks

@mistermaek
Copy link

I have this problem as well and needs this very urgent. Any ideas? Thanks in advance.
Does somebody has any experience in getting websockets running with angular6 and spring boot websocket on server side? Any tips will be very appreciated.
Thx.

@michaelmarshall
Copy link

michaelmarshall commented Jun 10, 2018

I have websockets working with Angular 6 and Spring Boot over sockjs and stomp.

One way to clear up this sockjs issue is to remove sockjs from your Angular app and use a cdn instead. Uninstall sockjs then add this to your index:
<script src="https://cdnjs.cloudflare.com/ajax/libs/sockjs-client/1.1.4/sockjs.min.js"></script>

Then add this after the imports in an angular component you want to use sockjs in:
declare var SockJS;

This should let you follow the tutorials out their without any issues. Here are two tutorials I used to get Spring and Angular websockets working (using sockjs and stomp):

Angular + Spring
Spring

@mistermaek
Copy link

mistermaek commented Jun 10, 2018

@michaelmarshall ,
thanks a lot for the tip above it works. I didn't get the global undefined error anymore.
I tried the Spring tutorials you provided me and it work's not 100%. If i try this greeting spring tutorial i get an connection short time and than i get the following error message.

http://127.0.0.1:8080/socket/info?t=1528658649817 0 ()
scheduleTask @ zone.js:2969
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.scheduleTask @ zone.js:407
onScheduleTask @ zone.js:297
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.scheduleTask @ zone.js:401
push../node_modules/zone.js/dist/zone.js.Zone.scheduleTask @ zone.js:232
push../node_modules/zone.js/dist/zone.js.Zone.scheduleMacroTask @ zone.js:255
scheduleMacroTaskWithCurrentZone @ zone.js:1114
(anonymous) @ zone.js:3001
proto.(anonymous function) @ zone.js:1394
r.start @ abstract-xhr.js:132
(anonymous) @ abstract-xhr.js:21
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask @ zone.js:421
onInvokeTask @ core.js:3662
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask @ zone.js:420
push../node_modules/zone.js/dist/zone.js.Zone.runTask @ zone.js:188
push../node_modules/zone.js/dist/zone.js.ZoneTask.invokeTask @ zone.js:496
ZoneTask.invoke @ zone.js:485
timer @ zone.js:2054
setTimeout (async)
scheduleTask @ zone.js:2075
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.scheduleTask @ zone.js:407
onScheduleTask @ zone.js:297
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.scheduleTask @ zone.js:401
push../node_modules/zone.js/dist/zone.js.Zone.scheduleTask @ zone.js:232
push../node_modules/zone.js/dist/zone.js.Zone.scheduleMacroTask @ zone.js:255
scheduleMacroTaskWithCurrentZone @ zone.js:1114
(anonymous) @ zone.js:2090
proto.(anonymous function) @ zone.js:1394
r @ abstract-xhr.js:20
r @ xhr-cors.js:8
r @ info-ajax.js:19
r.getReceiver @ info-receiver.js:39
r.doXhr @ info-receiver.js:56
(anonymous) @ info-receiver.js:25
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask @ zone.js:421
onInvokeTask @ core.js:3662
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask @ zone.js:420
push../node_modules/zone.js/dist/zone.js.Zone.runTask @ zone.js:188
push../node_modules/zone.js/dist/zone.js.ZoneTask.invokeTask @ zone.js:496
ZoneTask.invoke @ zone.js:485
timer @ zone.js:2054
setTimeout (async)
scheduleTask @ zone.js:2075
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.scheduleTask @ zone.js:407
onScheduleTask @ zone.js:297
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.scheduleTask @ zone.js:401
push../node_modules/zone.js/dist/zone.js.Zone.scheduleTask @ zone.js:232
push../node_modules/zone.js/dist/zone.js.Zone.scheduleMacroTask @ zone.js:255
scheduleMacroTaskWithCurrentZone @ zone.js:1114
(anonymous) @ zone.js:2090
proto.(anonymous function) @ zone.js:1394
r @ info-receiver.js:24
r @ main.js:121
push../src/app/tests/inletoutletsignal/inletoutletsignal.component.ts.InletOutletSignalComponent.connect @ inletoutletsignal.component.ts:83
InletOutletSignalComponent @ inletoutletsignal.component.ts:35
createClass @ core.js:8996
createDirectiveInstance @ core.js:8881
createViewNodes @ core.js:10101
createEmbeddedView @ core.js:10009
callWithDebugContext @ core.js:11046
debugCreateEmbeddedView @ core.js:10547
push../node_modules/@angular/core/fesm5/core.js.TemplateRef
.createEmbeddedView @ core.js:8596
push../node_modules/@angular/core/fesm5/core.js.ViewContainerRef
.createEmbeddedView @ core.js:8462
push../node_modules/@angular/cdk/esm5/portal.es5.js.CdkPortalOutlet.attachTemplatePortal @ portal.es5.js:661
push../node_modules/@angular/cdk/esm5/portal.es5.js.BasePortalOutlet.attach @ portal.es5.js:298
(anonymous) @ tabs.es5.js:377
push../node_modules/rxjs/_esm5/internal/Subscriber.js.SafeSubscriber.__tryOrUnsub @ Subscriber.js:253
push../node_modules/rxjs/_esm5/internal/Subscriber.js.SafeSubscriber.next @ Subscriber.js:191
push../node_modules/rxjs/_esm5/internal/Subscriber.js.Subscriber._next @ Subscriber.js:129
push../node_modules/rxjs/_esm5/internal/Subscriber.js.Subscriber.next @ Subscriber.js:93
push../node_modules/rxjs/_esm5/internal/operators/mergeMap.js.MergeMapSubscriber.notifyNext @ mergeMap.js:136
push../node_modules/rxjs/_esm5/internal/InnerSubscriber.js.InnerSubscriber._next @ InnerSubscriber.js:20
push../node_modules/rxjs/_esm5/internal/Subscriber.js.Subscriber.next @ Subscriber.js:93
(anonymous) @ subscribeTo.js:16
subscribeToResult @ subscribeToResult.js:6
push../node_modules/rxjs/_esm5/internal/operators/mergeMap.js.MergeMapSubscriber._innerSub @ mergeMap.js:127
push../node_modules/rxjs/_esm5/internal/operators/mergeMap.js.MergeMapSubscriber._tryNext @ mergeMap.js:124
push../node_modules/rxjs/_esm5/internal/operators/mergeMap.js.MergeMapSubscriber._next @ mergeMap.js:107
push../node_modules/rxjs/_esm5/internal/Subscriber.js.Subscriber.next @ Subscriber.js:93
(anonymous) @ subscribeToArray.js:9
push../node_modules/rxjs/_esm5/internal/Observable.js.Observable._trySubscribe @ Observable.js:176
push../node_modules/rxjs/_esm5/internal/Observable.js.Observable.subscribe @ Observable.js:161
push../node_modules/rxjs/_esm5/internal/operators/mergeMap.js.MergeMapOperator.call @ mergeMap.js:80
push../node_modules/rxjs/esm5/internal/Observable.js.Observable.subscribe @ Observable.js:158
push../node_modules/@angular/material/esm5/tabs.es5.js.MatTabBodyPortal.ngOnInit @ tabs.es5.js:375
checkAndUpdateDirectiveInline @ core.js:8945
checkAndUpdateNodeInline @ core.js:10209
checkAndUpdateNode @ core.js:10171
debugCheckAndUpdateNode @ core.js:10804
debugCheckDirectivesFn @ core.js:10764
(anonymous) @ MatTabBody.html:1
debugUpdateDirectives @ core.js:10756
checkAndUpdateView @ core.js:10153
callViewAction @ core.js:10394
execComponentViewsAction @ core.js:10336
checkAndUpdateView @ core.js:10159
callViewAction @ core.js:10394
execEmbeddedViewsAction @ core.js:10357
checkAndUpdateView @ core.js:10154
callViewAction @ core.js:10394
execComponentViewsAction @ core.js:10336
checkAndUpdateView @ core.js:10159
callViewAction @ core.js:10394
execComponentViewsAction @ core.js:10336
checkAndUpdateView @ core.js:10159
callViewAction @ core.js:10394
execEmbeddedViewsAction @ core.js:10357
checkAndUpdateView @ core.js:10154
callViewAction @ core.js:10394
execComponentViewsAction @ core.js:10336
checkAndUpdateView @ core.js:10159
callWithDebugContext @ core.js:11046
debugCheckAndUpdateView @ core.js:10724
push../node_modules/@angular/core/fesm5/core.js.ViewRef
.detectChanges @ core.js:8540
(anonymous) @ core.js:4429
push../node_modules/@angular/core/fesm5/core.js.ApplicationRef.tick @ core.js:4429
(anonymous) @ core.js:4322
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invoke @ zone.js:388
onInvoke @ core.js:3671
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.invoke @ zone.js:387
push../node_modules/zone.js/dist/zone.js.Zone.run @ zone.js:138
push../node_modules/@angular/core/fesm5/core.js.NgZone.run @ core.js:3585
next @ core.js:4322
schedulerFn @ core.js:3403
push../node_modules/rxjs/_esm5/internal/Subscriber.js.SafeSubscriber.__tryOrUnsub @ Subscriber.js:253
push../node_modules/rxjs/_esm5/internal/Subscriber.js.SafeSubscriber.next @ Subscriber.js:191
push../node_modules/rxjs/_esm5/internal/Subscriber.js.Subscriber._next @ Subscriber.js:129
push../node_modules/rxjs/_esm5/internal/Subscriber.js.Subscriber.next @ Subscriber.js:93
push../node_modules/rxjs/_esm5/internal/Subject.js.Subject.next @ Subject.js:53
push../node_modules/@angular/core/fesm5/core.js.EventEmitter.emit @ core.js:3395
checkStable @ core.js:3640
onHasTask @ core.js:3684
push../node_modules/zone.js/dist/zone.js.ZoneDelegate.hasTask @ zone.js:441
push../node_modules/zone.js/dist/zone.js.ZoneDelegate._updateTaskCount @ zone.js:461
push../node_modules/zone.js/dist/zone.js.Zone._updateTaskCount @ zone.js:285
push../node_modules/zone.js/dist/zone.js.Zone.runTask @ zone.js:205
drainMicroTaskQueue @ zone.js:595
push../node_modules/zone.js/dist/zone.js.ZoneTask.invokeTask @ zone.js:500
invokeTask @ zone.js:1540
globalZoneAwareCallback @ zone.js:1566

stomp.min.js:8 Whoops! Lost connection to http://127.0.0.1:8080/socket

It seems to me that my spring boot is not really there for the connection although i didn't receive a 404 so at least the url is correct and someone is listening on this port. Does the spring boot websocketconfig needs any additional infos?

@mistermaek
Copy link

Update.
It works now. I was so blind. I had a proxy configured because of CORS stuff and said that a specific api url will be forwarded. Added the api name in front of the api call of the spring example and it works.

var socket = new SockJS('/{myApiName}/gs-guide-websocket');

Thank you so much for pointing me in the right direction.

@tcjcodes
Copy link

For anyone still running into Webpack errors, this worked for me in Webpack v4.8.3:

module: {
   rules: [
   {
    test: require.resolve('sockjs-client'),
    use: [
        {
            /**
             * sockjs expects variable named 'global' to behave like window
             * @see {@link https://webpack.js.org/loaders/imports-loader}
             */
            loader: 'imports-loader',
            options: 'global=>window',
        },
        {
            /**
             * Expose sockjs module as `SockJs` on the window
             * @see {@link https://webpack.js.org/loaders/expose-loader/}
             */
            loader: 'expose-loader',
            options: 'SockJS'
        }
    ]
},

@brycekahle
Copy link
Contributor

To summarize, this is only a problem if you are bundling your own version of this library. There are pre-bundled versions available on CDNs and in the dist folder on npm (since 1.1.2).

I'd like to hear more about why folks are doing the bundling themselves, instead of using the pre-bundled versions. There are optimizations done for the production build, that you would miss out on, unless you duplicated that configuration.

@Andarist
Copy link
Author

Andarist commented Oct 7, 2018

From my perspective - it's also a slight problem because of the reason mentioned in the starting post of this discussion:

Having a global variable is not guaranteed! I.e. with webpack3 we can specify node: false option to stop any node specific polyfills being added to the bundle - this makes global undefined and thus this lib stops working.

I'm consuming sockjs-client directly from npm with webpack, which is super common nowadays. Less and less people are using prebundled from CDNs.

@brycekahle
Copy link
Contributor

I'm consuming sockjs-client directly from npm with webpack

So use the dist folder that is published to npm? Why do you need to bundle sockjs yourself?

@Andarist
Copy link
Author

Andarist commented Aug 3, 2020

Yes, that's an option - but nowadays you have to educate people to do this. The community expectation is that they can just require/import a package without accessing any deep paths. This flow is just easier because you can use all libraries like this without checking what path you need to access for each, so it just scales better. As you can see quite some people were confused by this, looking at comments and issues/PRs referencing this issue.

@github-actions
Copy link

github-actions bot commented Sep 3, 2020

This issue has been inactive for 30 days. It will be in closed in 5 days without any new activity.

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

No branches or pull requests

10 participants