Update JS syntax to Baseline 2025 using esupgrade - #2346
Conversation
matthiask
left a comment
There was a problem hiding this comment.
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.
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 ☂️ |
Thank you; <3 I wasn't even done yet.
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. |
c1277c7 to
147c2a4
Compare
e8c0d66 to
fd9b0ee
Compare
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 |
|
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. |
|
Not sure if and what to write in the change notes… |
|
Reviewing the recent discussions, the |
Maybe "Upgraded the JavaScript code to use modern ECMAScript features using esupgrade" or something like that?
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. |
|
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 don't have a strong preference. In reality, |
|
|
@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 |
|
Sounds good to me. |
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. |
|
@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. |
|
Oh btw, esupgrade uses CalVer. The date is the upper bounds feature wise. Of course within the realm of Baseline. |
|
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. |
|
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 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. |
|
Thank you for that clarification, that helped. |
|
@codingjoe can you give us permission to push to your branch please? |
Of course, done :) |
|
Thanks! |

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:
docs/changes.rst.AI/LLM Usage