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

TypeScript typings pollute global namespace #668

Open
lostfictions opened this issue Jul 13, 2020 · 1 comment
Open

TypeScript typings pollute global namespace #668

lostfictions opened this issue Jul 13, 2020 · 1 comment

Comments

@lostfictions
Copy link

Hi there, I noticed the typings include a global variable declaration at the bottom:

Sugar/sugar.d.ts

Line 1235 in 3ca5781

declare var Sugar: sugarjs.Sugar;

I can understand the reasoning behind including this line -- it might seem like the most expedient and convenient way to make sure those using Sugar as a global get the benefits of typings. Unfortunately, it irreversibly prevents all consumers of this library from having a variable named Sugar anywhere in their code. This is even true for consumers of a package that depends on Sugar -- if Sugar exists anywhere in their chain of dependencies, they're prevented from having a variable named Sugar. What's more, they'll always receive autocomplete suggestions in their tooling to use this global var Sugar, which can lead to runtime undefined errors; it also prevents auto-import tooling in editors like VSCode from working properly.

Given that that vast majority of people who've adopted TypeScript today are not using globals, this approach seems to hurt more than it helps, and it's out of line with best practices for typings. I'd suggest removing this line and instead recommending anyone using TS who needs Sugar as a global to make it accessible themselves via their own .d.ts file that declares that global itself; the handful of TS users working with legacy unscoped codebases should be familiar with how to do this.

@andrewplummer
Copy link
Owner

So v3 is currently being worked on that will use regular ES6 modules that do not expose the Sugar object as a global. As part of this the typings are being re-written from the ground up so this should be solved there. As for v2 this accurately represents the fact that Sugar is in fact exposed as a global so I think this is the correct typings for them no? If you have another suggestion about how to correctly type globals with TS I could change it as a patch for v2.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants