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

Jump to bottom

Support globalThis #12670

Closed
Standard8 opened this issue Dec 16, 2019 · 13 comments
Closed

Support globalThis #12670

Standard8 opened this issue Dec 16, 2019 · 13 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
v7.0.0

Comments

@Standard8
Copy link
Contributor

The version of ESLint you are using. 6.7.2

The problem you want to solve.

globalThis is now a tc 39 stage 4 proposal. ESLint should now be updated to recognise this.

Your take on the correct solution to problem.

Update the environment list to add globalThis

Are you willing to submit a pull request to implement this change? Yes
@Standard8 Standard8 added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Dec 16, 2019
Standard8 added a commit to Standard8/eslint that referenced this issue Dec 16, 2019
@Standard8
Add 'globalThis' to the list of es2020 globals (eslint#12670)
275d516
@Standard8
Copy link
Contributor Author

I've created #12671 for basic support of the global in the environment. Now I think about it a little more, I'm not sure if there's more follow-up work that ESLint will need to do to understand variables created via globalThis.

@kaicataldo kaicataldo added accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Dec 16, 2019
@kaicataldo
Copy link
Member

Agreed that we should update this. We'll also have to check and see if we need to update any of our core rules to account for this.

@mysticatea mysticatea added this to Issues which have PR in v7.0.0 Dec 17, 2019
@mysticatea
Copy link
Member

I added this to 7.0.0 board because I think this is a breaking change (e.g., no-redeclare may raise new warnings).

There are several rules we need to update. For example, the rules that check window.* should check globalThis.* as well.

@kaicataldo kaicataldo added the breaking This change is backwards-incompatible label Dec 17, 2019
@mysticatea mysticatea moved this from Issues which have PR to Accepted, ready to implement in v7.0.0 Dec 23, 2019
@yeonjuan
Copy link
Member

yeonjuan commented Dec 24, 2019 •
edited

Is someone working on it? :)

@mdjermanovic
Copy link
Member

@mysticatea some of the rules you listed in this comment are checking window.* via ReferenceTracker.

Should these rules now explicitly pass "globalThis" in options.globalObjectNames (along with the current defaults "global", "self", and "window"), or should this be an update in eslint-utils to add globalThis to the default list for options.globalObjectNames?

@mysticatea
Copy link
Member

@yeonjuan Probably not.

I published an eslint-utils update for globalThis.

@yeonjuan
Copy link
Member

Ok, I'll try it. :)

@mdjermanovic
Copy link
Member

PR #12774

@mysticatea mysticatea moved this from Accepted, ready to implement to Issues which have PR in v7.0.0 Jan 14, 2020
kaicataldo pushed a commit that referenced this issue Mar 17, 2020
@yeonjuan
Update: support globalThis (refs #12670) (#12774)
183e300 
* support globalThis in no-alert (ref #12670)

* support globalThis in no-eval (ref #12670)

* bump eslint-utils to ^2.0.0 (ref #12670)

* add globalThis test cases in require-unicode-regexp (ref #12670)

* add globalThis test cases in prefer-regex-literals (ref #12670)

* add globalThis test cases in prefer-object-spread (ref #12670)

* add globalThis test cases in prefer-named-capture-group (ref #12670)

* add globalThis test cases in prefer-exponentiation-operator (ref #12670)

* add globalThis test cases in no-misleading-character-class (ref #12670)

* edit test cases in no-eval

* support globalThis in no-obj-calls (ref #12670)

* add globalThis test cases in no-redeclare (#12670)

* change to use getPropertyName

* fix tpo

* add messageID - unexpectedRefCall
anikethsaha pushed a commit to anikethsaha/eslint that referenced this issue Mar 23, 2020
@yeonjuan @anikethsaha
Update: support globalThis (refs eslint#12670) (eslint#12774)
d21b2dc 
* support globalThis in no-alert (ref eslint#12670)

* support globalThis in no-eval (ref eslint#12670)

* bump eslint-utils to ^2.0.0 (ref eslint#12670)

* add globalThis test cases in require-unicode-regexp (ref eslint#12670)

* add globalThis test cases in prefer-regex-literals (ref eslint#12670)

* add globalThis test cases in prefer-object-spread (ref eslint#12670)

* add globalThis test cases in prefer-named-capture-group (ref eslint#12670)

* add globalThis test cases in prefer-exponentiation-operator (ref eslint#12670)

* add globalThis test cases in no-misleading-character-class (ref eslint#12670)

* edit test cases in no-eval

* support globalThis in no-obj-calls (ref eslint#12670)

* add globalThis test cases in no-redeclare (eslint#12670)

* change to use getPropertyName

* fix tpo

* add messageID - unexpectedRefCall
@btmills
Copy link
Member

btmills commented Mar 26, 2020

@yeonjuan thank you for implementing most of this in #12774! I realize we merged it before #12775 so you didn't have a chance to add globalThis support to no-implied-eval now that you've updated it. Would you like to update that last rule to fully close that issue? If you do, feel free to ping me in the PR so I'll get an email and can review it quickly.

yeonjuan added a commit that referenced this issue Mar 27, 2020
@yeonjuan
Update: support globalThis in no-implied-eval (fixes #12670)
fb11cab
@yeonjuan yeonjuan mentioned this issue Mar 27, 2020
Merged
2 tasks
@yeonjuan
Copy link
Member

@btmills
Sure : ). PR #13105 will support globalThis in no-implied-eval

v7.0.0 automation moved this from Issues which have PR to Done Mar 27, 2020
@janosh
Copy link

janosh commented Jun 4, 2020

I'm still seeing 'globalThis' is not defined. eslint(no-undef) in v7.1.

From reading the release notes, I thought support for globalThis landed in v7. Am I missing something?

@kaicataldo
Copy link
Member

Do you have the es2020 environment enabled?

@janosh
Copy link

janosh commented Jun 4, 2020

@kaicataldo Ah, that was the missing piece. Thanks a lot!

@kripod kripod mentioned this issue Jul 30, 2020
Merged
2 tasks
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Sep 24, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
No open projects
v7.0.0
  
Done
Milestone
No milestone
Development

No branches or pull requests
7 participants
@Standard8 @btmills @mysticatea @kaicataldo @janosh @yeonjuan @mdjermanovic