The Wayback Machine - https://web.archive.org/web/20220713004648/https://github.com/microsoft/TypeScript/pull/49027
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

Automatically enable debugMode when recording with Replay #49027

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented May 9, 2022

I'm not sure if you have heard about Replay (https://www.replay.io/) and I will totally understand if you decide to reject this PR.

However, the cost is small and it would make my life way easier :p as I'm using Replay to debug TypeScript quite a bit lately. I also highly encourage anyone interested in trying it out as it makes diagnosing and exploring code much easier. For the context - it is a time travel debugger that actually works with arbitrary scripts 😉

@typescript-bot typescript-bot added the For Uncommitted Bug label May 9, 2022
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented May 9, 2022

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@Andarist Andarist force-pushed the enable-debug-mode-in-replay branch from 0c4a6b5 to 14d9488 Compare May 9, 2022
@RyanCavanaugh RyanCavanaugh requested a review from rbuckton May 12, 2022
@RyanCavanaugh
Copy link
Member

@RyanCavanaugh RyanCavanaugh commented May 12, 2022

It's a little annoying that we have to probe process like that. Is there any other signal?

@Andarist
Copy link
Contributor Author

@Andarist Andarist commented May 12, 2022

what kind of an automated signal could we probe? I mean - is there even any other way to accomplish this than to probe a global (either directly checking something on process, or similar, or process.env)? I think that the Replay team could be open to introducing something else - but I truly don't know what else can be used to signal this.

@RyanCavanaugh
Copy link
Member

@RyanCavanaugh RyanCavanaugh commented May 12, 2022

I was hoping for an environment variable 😅

@Andarist
Copy link
Contributor Author

@Andarist Andarist commented May 12, 2022

Is there any particular reason why you are more OK with env var than with a process property? For context - I originally asked them if there is any env var that could be used to detect this because that was intuitively what I was expecting to be exposed. That's how I've learned that there is this process property available.

So my intuition was kinda like yours - but I don't mind any of those ways to detect something. It seems that this global object (process.recordreplay) has some additional methods exposed on it (like you can get a predicted ID of the recording, the current "pause point" and stuff like that). So from their PoV, it makes sense that this is an object on the process as they wouldn't be able to expose methods using env vars alone. So exposing additional env var could be seen as somewhat redundant from their PoV here as this global object will exist anyway.

If you are strongly opposed to checking this global object then I can circle back to the Replay team and ask if there is any chance that they would add some env var but I wonder if that's truly necessary.

@sandersn sandersn added this to Not started in PR Backlog May 18, 2022
@sandersn sandersn added the Housekeeping label May 18, 2022
@sandersn sandersn moved this from Not started to Waiting on reviewers in PR Backlog May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug Housekeeping
Projects
PR Backlog
  
Waiting on reviewers
Development

Successfully merging this pull request may close these issues.

None yet

5 participants