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

Help debuggers by not emitting loop bodies in new functions/classes? #61111

Open
6 tasks done
OfekShilon opened this issue Feb 4, 2025 · 5 comments
Open
6 tasks done
Labels
Question An issue which isn't directly actionable in code

Comments

@OfekShilon
Copy link

πŸ” Search Terms

debugging debugger watch scope

βœ… Viability Checklist

⭐ Suggestion

Here is a typical situation while debugging TS code:
Image

(in this case with chrome devtools).
A variable whose scope is larger than a certain loop (in this case even class scope this) is properly tracked through transpiling, but once breaking inside the loop - it appears to the debugger as undefined.

In this recent discussion in chromium issues, DevTools dev said that

in the Typescript source, the two breakpoints are in the same function. In the Javascript output, the breakpoints are in two different functions because the toolchain created a helper function, in which this is undefined.

This is actually a recurring complaint among debugger users: https://issues.chromium.org/issues/387132016, https://issues.chromium.org/issues/380459170

Are this extra-scopes really necessary? Can't the transpiler do its work within the original function?
If indeed necessary, perhaps source-maps can be somehow enriched to account for them?

Figuring this out would be of great value to all thos doing TS debugging out there.

πŸ“ƒ Motivating Example

Copying the repro steps from the devtools issue (captured in the gif above):

  1. Navigate to www.compiler-explorer.com
  2. Open DevTools and set breakpoints at history-eidget.ts, at lines 136 & 137 (see screenshot)
  3. In the site, click 'More'/'History' to reach the 1st breakpoint (see screenshot)
  4. When you'd break at the 1st breakpoint (outside the loop), this appears valid. Click F8 or 'continue' to reach the 2nd breakpoint (inside the loop), and observe that 'this' appears undefined

But this is an extremely common pain when debugging TS. No doubt you've experienced it yourselves if you ever debugged anything TS.

πŸ’» Use Cases

  1. What do you want to use this for?
    Debugging
  2. What shortcomings exist with current approaches?
    Unable to debug within loops
  3. What workarounds are you using in the meantime?
    printf-debugging
@OfekShilon OfekShilon changed the title Help debuggers by not emitting loop bodies in new functions/classes Help debuggers by not emitting loop bodies in new functions/classes? Feb 4, 2025
@RyanCavanaugh
Copy link
Member

Which emit are you referring to (i.e. what's the input code that's causing this)?

@OfekShilon
Copy link
Author

Again quoting the devtools discussion, TS source:

private hideRadiosAndSetDiff() {
        const root = unwrap(this.modal).find('.historiccode');
        const items = root.find('li:not(.template)');

        let foundbase = false;
        let foundcomp = false;

        for (const elem of items) {          // breakpoint 1
            const li = $(elem);              // breakpoint 2
            const dt = li.data('dt');

            const base = li.find('.base');
            const comp = li.find('.comp');

            let baseShouldBeVisible = true;
            let compShouldBeVisible = true;

            if (comp.prop('checked')) {
                foundcomp = true;
                baseShouldBeVisible = false;

                const itemRight = this.currentList.find(item => item.dt === dt);
                if (itemRight) {
                    unwrap(this.rhs).update(itemRight);
                }
            } else if (base.prop('checked')) {
                foundbase = true;

                const itemLeft = this.currentList.find(item => item.dt === dt);
                if (itemLeft) {
                    unwrap(this.lhs).update(itemLeft);
                }
            }

            if (foundbase && foundcomp) {
                compShouldBeVisible = false;
            } else if (!foundbase && !foundcomp) {
                baseShouldBeVisible = false;
            }

            if (compShouldBeVisible) {
                comp.css('visibility', '');
            } else {
                comp.css('visibility', 'hidden');
            }

            if (baseShouldBeVisible) {
                base.css('visibility', '');
            } else {
                base.css('visibility', 'hidden');
            }
        }
    }

Uglified transpiled JS:

e.prototype.hideRadiosAndSetDiff = function() {
                var e, t, i = (0,
                l.unwrap)(this.modal).find(".historiccode").find("li:not(.template)"), r = !1, s = !1, a = function(e) {
                    var t = (0,
                    n.default)(e)        // breakpoint 2
                      , i = t.data("dt")
                      , o = t.find(".base")
                      , a = t.find(".comp")
                      , c = !0
                      , d = !0;
                    if (a.prop("checked")) {
                        s = !0,
                        c = !1;
                        var p = u.currentList.find((function(e) {
                            return e.dt === i
                        }
                        ));
                        p && (0,
                        l.unwrap)(u.rhs).update(p)
                    } else if (o.prop("checked")) {
                        r = !0;
                        var g = u.currentList.find((function(e) {
                            return e.dt === i
                        }
                        ));
                        g && (0,
                        l.unwrap)(u.lhs).update(g)
                    }
                    r && s ? d = !1 : r || s || (c = !1),
                    d ? a.css("visibility", "") : a.css("visibility", "hidden"),
                    c ? o.css("visibility", "") : o.css("visibility", "hidden")
                }, u = this;
                try {
                    for (var c = o.__values(i), d = c.next(); !d.done; d = c.next()) {  // breakpoint 1
                        a(d.value)
                    }
                } catch (t) {
                    e = {
                        error: t
                    }
                } finally {
                    try {
                        d && !d.done && (t = c.return) && t.call(c)
                    } finally {
                        if (e)
                            throw e.error
                    }
                }
            }

@RyanCavanaugh
Copy link
Member

The function body here is necessary to get the correct let scoping semantics in ES5.

A really good alternative is to target ES6+, which virtually every runtime (and certainly any runtime running on a dev machine) supports

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Feb 4, 2025
@OfekShilon
Copy link
Author

OfekShilon commented Feb 5, 2025

The function body here is necessary to get the correct let scoping semantics in ES5.

A really good alternative is to target ES6+, which virtually every runtime (and certainly any runtime running on a dev machine) supports

Thank you for the suggestion, but I actually see zero diff in JS when changing tsconfig to "target": "es6". Even ESNext doesn't change anything.

Edit: Sorry, now it seems I can't build anything with ES6+.. will keep trying.

@MartinJohns
Copy link
Contributor

I actually see zero diff in JS when changing tsconfig to "target": "es6". Even ESNext doesn't change anything.

That's a configuration error on your side then (of whatever tooling you use besides TypeScript).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

3 participants