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

eslint: Bad use of globals #238

Open
1 task
Tracked by #235 ...
nexushoratio opened this issue Jan 31, 2024 · 1 comment
Open
1 task
Tracked by #235 ...

eslint: Bad use of globals #238

nexushoratio opened this issue Jan 31, 2024 · 1 comment
Labels
bug Something isn't working library For libraries linkedin-tool For the LinkedIn Tool userscript

Comments

@nexushoratio
Copy link
Owner

nexushoratio commented Jan 31, 2024

Apparently there are some JS globals in the browser environment that are commonly used locals, like name, self, toolbar, and event. So tools like eslint that know about them will not flag them with no-undef. As a result, at least one minor cosmetic bug exists: Shortcuts do not get their name property set because the same named local variable was not set.

Not yet sure what the proper solution is.

Setting no-shadow.builtinGlobals = true is one solution, but that just disallows us from reusing that name, but not from accidentally using it.

Setting globals.name = off forces using it locally, but we are still shadowing it, and difficult to know about them. Maybe there is a way to disallow certain variables, and somehow keep it in sync with known globals.

Also, seems to be at least 59 existing cases of reusing a global variable, mostly name.

Removing browser from the environment would expose 17 currently functions, and it means that testing new browser functions would require manually updating the list. Or, always go through globalThis for everything, and have it be the only known global property.

@nexushoratio nexushoratio added bug Something isn't working linkedin-tool For the LinkedIn Tool userscript library For libraries labels Jan 31, 2024
@nexushoratio nexushoratio self-assigned this Jan 31, 2024
nexushoratio pushed a commit that referenced this issue Feb 8, 2024
nexushoratio pushed a commit that referenced this issue Feb 8, 2024
Identify those currently being used so they can be fixed.

Issue #238.
nexushoratio pushed a commit that referenced this issue Feb 16, 2024
nexushoratio pushed a commit that referenced this issue Feb 18, 2024
@nexushoratio
Copy link
Owner Author

Dealing with name will be an interesting challenge.

One approach had been to disallow certain words as IDs. But, it turns out that eslint rule impacts not only variables, but field names as well. For those processed so far, was never an issue. But name is used all over the place. For instance, base.ensure().

nexushoratio pushed a commit that referenced this issue Feb 23, 2024
This change addresses local variable.

Issue #238.

␄
@nexushoratio nexushoratio removed their assignment Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working library For libraries linkedin-tool For the LinkedIn Tool userscript
Projects
None yet
Development

No branches or pull requests

1 participant