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

Discussion about global and experimental_enhanced #3860

Closed
mstoykov opened this issue Jul 18, 2024 · 3 comments
Closed

Discussion about global and experimental_enhanced #3860

mstoykov opened this issue Jul 18, 2024 · 3 comments
Assignees
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 js-compat question triage

Comments

@mstoykov
Copy link
Contributor

What?

extended apart from using babel (until recently) to add more ECMAScript support also adds global as an alias to globalThis

k6/js/bundle.go

Lines 387 to 392 in 111985b

if b.CompatibilityMode == lib.CompatibilityModeExtended {
err = rt.Set("global", rt.GlobalObject())
if err != nil {
return err
}
}

This was readded after the dropping of core-js #1845 and is currently the sole difference between extended and base.

experimental_enhanced doesn't add this alias.

Proposal:

Keep it this way.

I would argue that we should drop it from extended in the future as well, but this should be discussed separately.

experimental_enhanced doesn't have a history of supporting this, and we have had in the past had problems with this being used by libraries to detect if they are running in nodejs. Which then fails as it goes down a code path that isn't actually supported.

Nodejs and every other known to me engine supports globalThis and that is the standard way.

We could potentially revise this in the future if there are cases that come to light that this will be beneficial.

@mstoykov mstoykov added question js-compat evaluation needed proposal needs to be validated or tested before fully implementing it in k6 labels Jul 18, 2024
@szkiba
Copy link
Contributor

szkiba commented Jul 18, 2024

The proposal sounds reasonable to me.

@codebien
Copy link
Contributor

codebien commented Jul 19, 2024

If we don't have good reasons to keep it then I'm all for it, can we deprecate it just right now?

@mstoykov
Copy link
Contributor Author

I am closing this as it seems we have alignment on this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 js-compat question triage
Projects
None yet
Development

No branches or pull requests

4 participants