Skip to content

[wasm] make debugger less noisy - #72675

Closed
pavelsavara wants to merge 1 commit into
dotnet:mainfrom
pavelsavara:wasm_debugger_ready
Closed

[wasm] make debugger less noisy#72675
pavelsavara wants to merge 1 commit into
dotnet:mainfrom
pavelsavara:wasm_debugger_ready

Conversation

@pavelsavara

@pavelsavarapavelsavara commented Jul 22, 2022•
edited
Loading

Copy link
Copy Markdown
Member

Because it matters in shell or NodeJS #63703

@pavelsavarapavelsavara added arch-wasm WebAssembly architecture area-Debugger-mono labels Jul 22, 2022
@pavelsavarapavelsavara added this to the 7.0.0 milestone Jul 22, 2022
@ghost

ghost commented Jul 22, 2022

Copy link
Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Because it matters in shell or NodeJS

Author:pavelsavara
Assignees:pavelsavara, thaystg
Labels:

arch-wasm, area-Debugger-mono

Milestone:7.0.0

lewing
lewing reviewed Jul 22, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/debug.ts
debugger;
else
console.debug("mono_wasm_runtime_ready", "fe00e07a-5519-4dfe-b35a-f867dbaf2e28");
console.assert(true, "mono_wasm_runtime_ready", "fe00e07a-5519-4dfe-b35a-f867dbaf2e28");

@lewinglewingJul 22, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is assert available everywhere?

@thaystgthaystgJul 22, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It does not work on firefox, trying to find another solution.

@pavelsavarapavelsavaraJul 22, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The browser API is everywhere. How it's reported to debugger protocol is different for FF.
https://caniuse.com/mdn-api_console_assert

radekdoulik
radekdoulik approved these changes Jul 23, 2022
View reviewed changes

@radekdoulikradekdoulik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

I wonder about the src/mono/wasm/runtime/package-lock.json file. Do we need it in the repo?

@pavelsavara

pavelsavara commented Jul 25, 2022

Copy link
Copy Markdown
MemberAuthor

LGTM

I wonder about the src/mono/wasm/runtime/package-lock.json file. Do we need it in the repo?

Yes, we do need it. The problem is that emsdk has older version of nodeJS than most of us have on the system.
When I did package upgrade last week, we I used node on my system, which generated lock file version 2 (backward compatible with v1). But when we run the same command as part of build, it would run npm with smsdk node version and that would produce the lock file in v1 format and create this delta. I guess it's better to commit the delta and stick with v1 lock file, than to fight it on each commit.

@pavelsavara

pavelsavara commented Jul 26, 2022

Copy link
Copy Markdown
MemberAuthor

Closing in favor of #72812

@pavelsavara
pavelsavara deleted the wasm_debugger_ready branch July 29, 2022 09:16
@ghostghost locked as resolved and limited conversation to collaborators Aug 28, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lewinglewinglewing left review comments
@thaystgthaystgthaystg left review comments
@radekdoulikradekdoulikradekdoulik approved these changes
Copilot code reviewCopilot
Copilot will automatically review once the pull request is marked ready for review

Assignees

@pavelsavarapavelsavara

@thaystgthaystg

Labels

arch-wasmWebAssembly architecturearea-Debugger-mono

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@pavelsavara@lewing@radekdoulik@thaystg