feat(utils): Modern implementation of getGlobalObject

[timfish]

This PR:

• Replaces getGlobalObject global object detection with a modern implementation taken from core-js which is then modified/simplified a little
• Prioritises globalThis which was previously removed due to requiring a TypeScript version which was required in a no longer supported version of Angular (ERROR in node_modules/@sentry/browser/dist/transports/base.d.ts(31,19): error TS2503: Cannot find namespace 'globalThis'. #2978)
  • This is a requirement for future support of other js runtimes (Support different JS runtimes #5611)
• No longer uses isNodeEnv 🎉
• Caches the global so it doesn't need to be evaluated for every invocation of getGlobalObject

[AbhiPrasad]

I wonder if we can also clean up our usage of getGlobalObject around the codebase while we do this.

edit: The Node 8 test failures 😓

[timfish]

The node test failures were due to me assuming I knew better than the many hours that have been put into the core-js code 🤷‍♂️

The ember test failure look unrelated?

I wonder if we can also clean up our usage of getGlobalObject around the codebase while we do this.

There might be some options there for cleanup but it will be much easier to revert this PR if it only contains this one change 😂

[AbhiPrasad]

The ember test failure look unrelated?

Yeah it's probably a flake

There might be some options there for cleanup but it will be much easier to revert this PR if it only contains this one change 😂

Yeah, let's do it in another change. We can even make a new task to track that!

[lforst]

A classic 🤣

[lobsterkatie]

A few small suggestions for slightly easier reading, but otherwise LGTM!

(Oh, actually, one half-baked thought/question: Is there a reason we couldn't just do away with getGlobalObject() entirely and just import GLOBAL everywhere? I suppose we'd lose the typecasting we get from the generic (when we use it, which we don't always), but getGlobalObject<Widow>() vs GLOBAL as Window is kind of a wash on that score. Maybe it could be an issue with web workers?

Anyway, not saying it's necessarily better, just something to think about.)

[timfish]

A few small suggestions for slightly easier reading, but otherwise LGTM!

Agree they're all much clearer. Also renamed the check function to isGlobalObj and with a more sensible param name.

Is there a reason we couldn't just do away with getGlobalObject() entirely and just import GLOBAL everywhere? I supposed we'd lose the typecasting we get from the generic (when we use it, which we don't always), but getGlobalObject() vs GLOBAL as Window is kind of a wash on that score. Maybe it could be an issue with web workers?

I would be tempted to hide the types behind different exports to reduce the use of generics everywhere and then eventually move them to the individual runtime packages:

export const GLOBAL_OBJ: SentryGlobal = /* get the global object here */;

// This would eventually end up in `@sentry/browser`
export const WINDOW = GLOBAL_OBJ as Window & SentryGlobal;

// This would eventually end up in `@sentry/node`
export const GLOBAL = GLOBAL_OBJ as NodeJs.Global & SentryGlobal;

// This would eventually end up in `@sentry/some-runtime`
export const GLOBAL_THIS = GLOBAL_OBJ as GlobalThis & SentryGlobal;

This might go against some of the reasoning here, however the current typecasting with getGlobalObject allows us to be quite sloppy about which packages we put code in. I would argue that if we want to enforce that the core packages are runtime agnostic, whatever we replace getGlobalObject<Window>() with, it should only be accessible in packages where window actually exists.

With the above code, we'd still have full runtime global object detection but the types would be be a lot more strict around different runtimes.

[lobsterkatie]

Ah, right, thanks for linking to that comment of Kamil's. Yeah, that's what I meant about it possibly being an issue for web workers - exactly what he mentions about them not having window defined. If we moved to only using your WINDOW constant in @sentry/browser, would that end up being a problem?

[timfish]

Yeah, that's what I meant about it possibly being an issue for web workers - exactly what he mentions about them not having window defined. If we moved to only using your WINDOW constant in @sentry/browser, would that end up being a problem?

My suggestion in that discussion was to just use window where it's available and there are issues with that.

However, WINDOW above would work with web workers as is just a typecasted reference of GLOBAL_OBJ which will end up being self in a web worker. It's functionally equivalent to getGlobalObject<Window>().

If we have a global type that we want to use in both browser and web worker contexts, we probably should create a union type to reflect what is available in both with optional properties for what is only available in one context. Using the Window type as the global in web workers has risks because that is not a true representation of the global scope there.

[AbhiPrasad]

I would be tempted to hide the types behind different exports to reduce the use of generics everywhere and then eventually move them to the individual runtime packages

I'm a big fan of this idea, can we try experimenting with this as a next step?

If we have a global type that we want to use in both browser and web worker contexts

I think we should generally assume that this is the case, and that either self or window will be returned, we should adjust the type there accordingly.

I'm going to go ahead and merge this because it'll help unblock the rest of the work we have to do here.