-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Cleanup comments and some code after ESM merge #3862
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with some non-blocking texting adjustments 👍
Co-authored-by: Oleg Bespalov <oleg.bespalov@grafana.com>
extended: base + Babel with parts of ES2015 preset | ||
slower to compile in case the script uses syntax unsupported by base | ||
base: pure Sobek - Golang JS VM supporting ES6+ | ||
extended: base + sets "global" as alias for "globalThis" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, what do we plan here? Do we drop in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good question - no answer at this point.
see #3860 for example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Ivan <2103732+codebien@users.noreply.github.com>
What?
Cleanup comments and messages around ESM merge
Why?
A lot of those comments no longer make sense with Babel gone and some refactoring of the internals
Checklist
make lint
) and all checks pass.make tests
) and all tests pass.Related PR(s)/Issue(s)