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

fix: Missing typescript issue check #981

Closed
wants to merge 4 commits into from
Closed

Conversation

benjackwhite
Copy link
Collaborator

@benjackwhite benjackwhite commented Jan 31, 2024

Changes

Locally pnpm build errors with a TS issue. Trying to recreate on the CI so we can fix and figure out why it wasn't caught...

Outcome: Couldn't figure out why it wasn't working but swapping to the window's setTimeout fixed it...

Checklist

Copy link

github-actions bot commented Jan 31, 2024

Size Change: +70 B (0%)

Total Size: 767 kB

Filename Size Change
dist/array.full.js 180 kB +19 B (0%)
dist/array.js 121 kB +17 B (0%)
dist/es.js 121 kB +17 B (0%)
dist/module.js 121 kB +17 B (0%)
ℹ️ View Unchanged
Filename Size
dist/exception-autocapture.js 12 kB
dist/recorder-v2.js 105 kB
dist/recorder.js 58.5 kB
dist/surveys.js 48.7 kB

compressed-size-action

@benjackwhite benjackwhite changed the title Fix build step fix: Missing typescript issue check Jan 31, 2024
@benjackwhite benjackwhite marked this pull request as ready for review January 31, 2024 12:18
@pauldambra
Copy link
Member

weird there's no error for me... I can run yalc publish and pnpm build successfully

what error do you get?

Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assuming the checks all pass....

@@ -619,7 +619,7 @@ export class SessionRecording {
clearInterval(this._fullSnapshotTimer)
}

this._fullSnapshotTimer = setInterval(() => {
this._fullSnapshotTimer = window?.setInterval(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels like a safe enough change so 🚢

but definitely be good to understand why this works in CI, and on my machine, but not on yours

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's surprising... I will clear my local node_modules or something and try again...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ha. The error went away!

@benjackwhite benjackwhite deleted the fix/build-issue branch January 31, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants