Skip to content

Update JS syntax to Baseline 2025 using esupgrade - #2346

Merged
matthiask merged 7 commits into
django-commons:mainfrom
codingjoe:esupgrade
May 4, 2026
Merged

Update JS syntax to Baseline 2025 using esupgrade#2346
matthiask merged 7 commits into
django-commons:mainfrom
codingjoe:esupgrade

Conversation

@codingjoe

@codingjoe codingjoe commented Apr 18, 2026

Copy link
Copy Markdown
Member

Description

I was browsing through the JS code and stumbled upon dated syntax. Since I wrote a tool for it, I ran it. However, I added more commits to manually address some discrepancies.

Checklist:

  • I have added the relevant tests for this change.
  • I have added an item to the Pending section of docs/changes.rst.

AI/LLM Usage

  • This PR includes code generated with the help of an AI/LLM

I got some LLM powered autocomplete help while writing the tests.

@github-actions

github-actions Bot commented Apr 18, 2026

Copy link
Copy Markdown

Coverage report

This PR does not seem to contain any modification to coverable code.

@matthiask matthiask 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.

For the record: I mostly agree with the changes here.

I don't really know about the change in debounce though. I think the function should either use promises or async/await, not both (without spending a lot of time thinking about this). Maybe I'm misunderstanding why the change is actually good, but I think I'd prefer the non-async version with an explicit promise.

Comment thread .github/workflows/test.yml Fixed
Comment thread .github/workflows/test.yml Fixed
Comment thread .github/workflows/test.yml Fixed
Comment thread .github/workflows/test.yml Fixed
Comment thread .github/dependabot.yml Fixed
Comment thread .github/workflows/test.yml Fixed
Comment thread .github/workflows/test.yml Fixed
@codecov

codecov Bot commented Apr 18, 2026

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

@codingjoe

Copy link
Copy Markdown
Member Author

For the record: I mostly agree with the changes here.

Thank you; <3 I wasn't even done yet.

I don't really know about the change in debounce though. I think the function should either use promises or async/await, not both (without spending a lot of time thinking about this). Maybe I'm misunderstanding why the change is actually good, but I think I'd prefer the non-async version with an explicit promise.

Yeah, I can attest that it takes some getting used to. Both versions are fully identical, though. Same as in Python, an async function is a promise/coroutine. As always, syntax is debatable, and we don't have to add esupgrade as a dependency to leave more wiggle room. That being said, it's designed to use ECMAScript's more recent features with full browser support that they have borrowed from Python. So the resulting code "should" be more pleasing to a Pythonista's eye.

That being said, I don't love the debounce implementation. Ideally it could handle both async and synchronous functions. I will propose a more sophisticated solution.

@codingjoe
codingjoe force-pushed the esupgrade branch 5 times, most recently from c1277c7 to 147c2a4 Compare April 18, 2026 18:58
Comment thread .github/workflows/test.yml Fixed
Comment thread .github/workflows/test.yml Fixed
Comment thread .github/workflows/test.yml Fixed
Comment thread .github/workflows/test.yml Fixed
Comment thread .github/workflows/test.yml Fixed
@codingjoe
codingjoe force-pushed the esupgrade branch 2 times, most recently from e8c0d66 to fd9b0ee Compare April 19, 2026 10:01
@matthiask

Copy link
Copy Markdown
Member

I don't really know about the change in debounce though. I think the function should either use promises or async/await, not both (without spending a lot of time thinking about this). Maybe I'm misunderstanding why the change is actually good, but I think I'd prefer the non-async version with an explicit promise.

Yeah, I can attest that it takes some getting used to. Both versions are fully identical, though. Same as in Python, an async function is a promise/coroutine. As always, syntax is debatable, and we don't have to add esupgrade as a dependency to leave more wiggle room. That being said, it's designed to use ECMAScript's more recent features with full browser support that they have borrowed from Python. So the resulting code "should" be more pleasing to a Pythonista's eye.

Yes, I understand that. I like automatically upgrading code using opinionated pre-commit hooks, so I'm good with using esupgrade. That said, when the debounce function does nothing else but construct a promise and immediately return it, making the function async and await-ing the promise in the function doesn't really buy us anything. I do use both async/await and Promise in my code, but do not immediately recall good reasons for using both async styles within the same function.

@tim-schilling

Copy link
Copy Markdown
Member

Can we split out the trivial changes from the non-trivial in a separate PR?

@codingjoe

Copy link
Copy Markdown
Member Author

Can we split out the trivial changes from the non-trivial in a separate PR?

Already did. Just didn't get around to rebasing it yet. Let me review it myself one more time, and we should be golden to move this along.

@codingjoe
codingjoe marked this pull request as ready for review April 22, 2026 16:49
@codingjoe
codingjoe requested a review from matthiask April 22, 2026 16:50
@codingjoe

Copy link
Copy Markdown
Member Author

Not sure if and what to write in the change notes…

@federicobond

Copy link
Copy Markdown
Member

Reviewing the recent discussions, the Promise.try reasoning made sense to me when I read it, but I'm worried that forcing a newly-available baseline will push us toward relying on too many recent features. Would widely-available work better here?

@matthiask

Copy link
Copy Markdown
Member

Not sure if and what to write in the change notes…

Maybe "Upgraded the JavaScript code to use modern ECMAScript features using esupgrade" or something like that?

Reviewing the recent discussions, the Promise.try reasoning made sense to me when I read it, but I'm worried that forcing a newly-available baseline will push us toward relying on too many recent features. Would widely-available work better here?

I just checked the esupgrade README and now I'm wondering about that as well. I think having to wait 30 months until we can use a feature is too long for the toolbar (when the feature really simplifies our code), but the low end of "available in all browsers for 0-30 months" means we're adopting new features really quickly. I wish there was a sweet spot somewhere in between. It's not necessarily a blocker for me but I can see how that's slightly uncomfortable. I probably wouldn't be affected because I'm definitely updating browsers quickly, but I generally try to lean towards for example supporting Firefox ESR and not just the latest versions.

@tim-schilling

Copy link
Copy Markdown
Member

It may be worth mentioning that while we assume developers would use the latest versions, doesn't mean they are. It's possible they work on a Django app which has a different window of support for their own customers, so they match their work browser to that of their customers.

@codingjoe

Copy link
Copy Markdown
Member Author

Reviewing the recent discussions, the Promise.try reasoning made sense to me when I read it, but I'm worried that forcing a newly-available baseline will push us toward relying on too many recent features. Would widely-available work better here?

I don't have a strong preference. In reality, Promise.try is currently the only supported transformation, and I introduced its usage manually. That being said, new APIs will be released. But it's a pretty high bar to clear (to give people like us confidence). Chrome (desktop and Android), Edge, Firefox (desktop and Android), Safari (macOS and iOS). Widely available just adds a 30-month cooldown.

@codingjoe

codingjoe commented Apr 22, 2026

Copy link
Copy Markdown
Member Author

It may be worth mentioning that while we assume developers would use the latest versions, doesn't mean they are. It's possible they work on a Django app which has a different window of support for their own customers, so they match their work browser to that of their customers.

@tim-schilling #2357 😉

@codingjoe

Copy link
Copy Markdown
Member Author

@matthiask @federicobond I reverted it back to widely available. If we want to use newer features, we can do so but should have a good reason. Like we Promise.try. Sounds sane?

@federicobond

Copy link
Copy Markdown
Member

Sounds good to me.

@matthiask

Copy link
Copy Markdown
Member

It may be worth mentioning that while we assume developers would use the latest versions, doesn't mean they are. It's possible they work on a Django app which has a different window of support for their own customers, so they match their work browser to that of their customers.

I'm pushing back a little in the spirit of strong convictions weakly held.

That was why I think Firefox ESR or something like this should be the baseline. I don't think older browsers are a large concern though. The current version of Django, 6.0, only supports Python 3.12 or better. 3.12 has been released at the end of 2023. Requiring a browser released only a few months later when it's generally much easier to update software on developer devices than on servers doesn't strike me as needlessly raising the bar too high.

I'm definitely still running Python 3.9 or even 2.7 somewhere where it's hard to upgrade. I'm not aware of people using old browsers except in environments where they really need(ed) ActiveX or maybe legacy Edge. Company IT departments are strange but especially they should know that running insecure old versions is a bad idea. I am going to claim without having much evidence that people will be aware of the fact that they are running outdated software in these environments.

What I do see as a problem is the fact that we cannot automatically downgrade the package to a version which matches the browser. Without upper bounds on our supported Django version we don't automatically select a supported django-debug-toolbar version either though, so maybe the difference isn't that big. (I'm not advocating for introducing them, I think upper bounds are generally a bad idea.)

That said: I don't want to block or veto anything related to greater backwards compatibilty, even if I could.

@codingjoe

Copy link
Copy Markdown
Member Author

@matthiask I think this is a very valid argument for NodeJs. But browsers, are messy, you'd need to pin dozens of versions and platforms. A percentile target was introduced as a practice in browserlist a long time ago. But it had issues too, since there was no clear recommendation.

The Baseline project was founded by the W3C to overcome this issue and provide a guidelines for the Internet to upgrade with confidence.

I am personally very happy to place some of the burden on the W3C. But milage may vary.

@codingjoe

Copy link
Copy Markdown
Member Author

Oh btw, esupgrade uses CalVer. The date is the upper bounds feature wise. Of course within the realm of Baseline.

@tim-schilling

tim-schilling commented Apr 23, 2026

Copy link
Copy Markdown
Member

I appreciate the idea of using Firefox ESR for this. I'm in agreement there. Though we probably want something that spans the major browsers. I think we can widely available is a reasonable indicator for us to use. That means it's about 6 months more than Firefox ESR for a reference point. This avoids us having to check all three browsers for when their latest LTS ends.

To help ground us in some numbers for possibilities. If there are a million Django projects out there (guess), the toolbar is installed for 15% of them (from pypi download ratios). That means we are used in 150,000 projects. That's going to mean a wide variety of scenarios.

@tim-schilling

Copy link
Copy Markdown
Member

What is this tool providing that biome doesn't? They seem fairly related to me, but I may be missing something. Is it possible for us to use biome differently to include this functionality so we use fewer tools and avoid potential conflicts?

@codingjoe

Copy link
Copy Markdown
Member Author

What is this tool providing that biome doesn't? They seem fairly related to me, but I may be missing something. Is it possible for us to use biome differently to include this functionality so we use fewer tools and avoid potential conflicts?

Biome => Ruff
esupgrade => pyupgrade

The former is a linter and formatter, the latter a syntax upgrader. Of course, both change syntax, but to a different end. I am aware that Biome has some similar transformers, but not nearly as complete; not all of them are safe.

That being said, I initially proposed NOT to include the pre-commit hook. Mainly to separate a linter discussion from the actual code changes.

@tim-schilling

Copy link
Copy Markdown
Member

Thank you for that clarification, that helped.

@tim-schilling

Copy link
Copy Markdown
Member

@codingjoe can you give us permission to push to your branch please?

@codingjoe

codingjoe commented May 2, 2026

Copy link
Copy Markdown
Member Author

@codingjoe can you give us permission to push to your branch please?

Of course, done :)

Comment thread debug_toolbar/static/debug_toolbar/js/utils.js
@matthiask
matthiask merged commit 405d968 into django-commons:main May 4, 2026
29 checks passed
@matthiask

Copy link
Copy Markdown
Member

Thanks!

@codingjoe

Copy link
Copy Markdown
Member Author
Happy Sesame Street GIF by Muppet Wiki

@codingjoe
codingjoe deleted the esupgrade branch May 4, 2026 08:41
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.

5 participants