-
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
Warn on mixing ESM and commonJS #3807
Conversation
As part of testing for #3265 it was noticed this isn't uncommon. As there is no way for this to be supported and it will be against how the specification works we prefer to warn users on this at least for a little while.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3807 +/- ##
=========================================
Coverage ? 71.87%
=========================================
Files ? 291
Lines ? 21274
Branches ? 0
=========================================
Hits ? 15290
Misses ? 4920
Partials ? 1064
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Co-authored-by: Théo Crevon <oleiade@users.noreply.github.com>
Co-authored-by: Ivan <2103732+codebien@users.noreply.github.com>
It seems that at some point when this was written someone mixed ESM and CommonJS syntax. There is not good reason to do this, and it will soon not be supported - and arguably has never been, it just happened to work due to how ESM was supported in k6. See grafana/k6#3807 and linked issues/PRs
It seems that at some point when this was written someone mixed ESM and CommonJS syntax. There is not good reason to do this, and it will soon not be supported - and arguably has never been, it just happened to work due to how ESM was supported in k6. See grafana/k6#3807 and linked issues/PRs
What?
As part of testing for #3265 it was noticed this isn't uncommon.
Why?
As there is no way for this to be supported and it will be against how the specification works we prefer to warn users on this at least for a little while.
Checklist
make lint
) and all checks pass.make tests
) and all tests pass.Related PR(s)/Issue(s)