Skip to content

Sentry typescript type error introduced in 7.16.0 #6016

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
3 tasks done
nahidakbar opened this issue Oct 24, 2022 · 11 comments · Fixed by #6024
Closed
3 tasks done

Sentry typescript type error introduced in 7.16.0 #6016

nahidakbar opened this issue Oct 24, 2022 · 11 comments · Fixed by #6024
Assignees

Comments

@nahidakbar
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/serverless

SDK Version

7.16.0

Framework Version

Serverless

Link to Sentry event

No response

Steps to Reproduce

For my GCP serverless codebase,

Nodejs v16.13.2

Typescript 4.8.3

if I upgrade my sentry packages from 7.15.0 to 7.16.0, it gives error

Expected Result

No error

Actual Result

node_modules/@sentry/utils/types/browser.d.ts:4:69 - error TS2304: Cannot find name 'Window'.
@AbhiPrasad
Copy link
Member

This is occuring because we have top level casting for Window, where previously it was gated behind a function call.

@timfish could we move these exports into the browser package to fix this?

@AbhiPrasad
Copy link
Member

ref #5831

@timfish
Copy link
Collaborator

timfish commented Oct 24, 2022

WINDOW is still used extensively in utils so moving it from there is not an option for now.

I guess the only option is to put it behind a function call for now?

export function WINDOW(): Window & InternalGlobal {
  return GLOBAL_OBJ as Window & InternalGlobal;
}

@AbhiPrasad
Copy link
Member

We'll probably have to cast at call site because top level call will still break TS definitions.

Why can't we move the WINDOW functions to @sentry/browser? The only file that is maybe a little tricky to do is time.ts, but that can be split up.

@timfish
Copy link
Collaborator

timfish commented Oct 24, 2022

WINDOW is currently used in:

  • browser.ts - getLocationHref and getDomElement could probably be moved to @sentry/browser
  • supports.ts - Much of this code can be moved to @sentry/browser too
  • instrument.ts - addInstrumentationHandler is used in @sentry/tracing. Not sure how to solve that.
  • time.ts - Tricky to split up since this uses perf_hooks on node.js. performance should almost certainly be passed down from the client like TextEncoder.

Earlier we had this code in utils with no issues:

export function getGlobalObject<T>(): T & SentryGlobal {
return (
isNodeEnv()
? global
: typeof window !== 'undefined' // eslint-disable-line no-restricted-globals
? window // eslint-disable-line no-restricted-globals
: typeof self !== 'undefined'
? self
: fallbackGlobalObject
) as T & SentryGlobal;
}

const global = getGlobalObject<Window>();

So Window WAS passed from the call site, but in many cases, the call site was still in utils!

@AbhiPrasad
Copy link
Member

Yeah instrument.ts and time.ts make this tricky. time.ts we maybe can just solve by just having a node specific time.ts and browser specific time.ts, and not bothering to dependency inject or keep a agnostic file, but instrument prob cannot be solved that way (unless we also inject the global maybe?).

So Window WAS passed from the call site, but in many cases, the call site was still in utils!

I think the issue here is that a top level def of export function WINDOW(): Window & InternalGlobal { will break. We'll maybe have to bring back getGlobalObject just for these functions.

@timfish
Copy link
Collaborator

timfish commented Oct 24, 2022

We'll maybe have to bring back getGlobalObject just for these functions.

Yeah, it doesn't need to be exported and can be used only internally in utils

@timfish
Copy link
Collaborator

timfish commented Oct 24, 2022

it doesn't need to be exported and can be used only internally in utils

Oh, that is not the case since we can't keep WINDOW in utils.

Do you think a generic will work with a default type?

export function WINDOW<T = Window>(): InternalGlobal & T {
  return GLOBAL_OBJ as InternalGlobal & T;
}

@AbhiPrasad
Copy link
Member

Do you think a generic will work with a default type?

I don't think so.

Oh, that is not the case since we can't keep WINDOW in utils.

We can still keep it as long as it's within a function call right? We don't have to use the default generic, just leave it as T, same as getGlobalObject.

@dgesteves
Copy link

dgesteves commented Mar 23, 2023

I am getting this error in Production and need help fixing it as soon as possible:
`import { GLOBAL_OBJ } from '@sentry/utils';

const WINDOW = GLOBAL_OBJ ;

export { WINDOW };
//# sourceMappingURL=types.js.map
`

"@sentry/react": "^7.44.2",
"@sentry/tracing": "^7.44.2",
"@sentry/vite-plugin": "^0.4.0",

Screenshot 2023-03-23 at 19 56 21

@timfish
Copy link
Collaborator

timfish commented Mar 23, 2023

@dgesteves this is a different issue. Please open a new one.

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

Successfully merging a pull request may close this issue.

4 participants