Skip to content

compiler option to enforce that declare global or declare module only happens when those files are imported. #42003

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

Closed
5 tasks done
trusktr opened this issue Dec 17, 2020 · 5 comments
Labels
Needs More Info The issue still hasn't been fully clarified

Comments

@trusktr
Copy link
Contributor

trusktr commented Dec 17, 2020

I think this would be a very powerful option that would solve issues like #37053 (in particular see the list of related at the bottom of the comment #37053 (comment)).

Suggestion

A new option, like declareOnImportOnly would be amazing!

Use Cases

Given a file like

// somelib/FooBar.ts
class FooBar extends HTMLElement {
  foo() {console.log('foo')}
}
customElements.define('foo-bar', FooBar)
declare global {
    interface HTMLElementTagNameMap {
        'foo-bar': FooBar
    }
}

If the user installs somelib, but they never import the somelib/FooBar.ts file (or somelib/FooBar.{js/d.ts} if the package is compiled), they will still have the definition of 'foo-bar': FooBar.

If the user writes the following code, they will not see a type error, but they will have an unexpected runtime error:

// el's type is implicitly FooBar:
const el = document.createElement('foo-bar') // no error here, but there should be

// No type error here (but there should be). The user believes this will work:
el.foo()

At runtime, the user will get an error like cannot call 'undefined' because the foo-bar element was never defined, because that file was never imported, therefore el.foo is not a function because el is an instance of HTMLElement and not an instance of FooBar.

For the users code to work, they need to import the file (directly or indirectly):

// without this import statement, the `foo-bar` element would not be augmented onto `HTMLElementTagNameMap`
import 'somelib/FooBar' 

const el = document.createElement('foo-bar') // ok!
el.foo() // ok!

For all the problems listed in #37053, the solution is simple: with the new declareOnImportOnly option set to true, you must simply import whatever you need.

This would be great, because the purpose of import syntax is fulfilled: now user use import for importing... anything they need!

No longer will unexpected types appear in scope (or at least, it will be minimized, because if a user imports a package's index file, they may end up importing all the types that they didn't want, but at least they won't get runtime errors because the runtime modules will have also been imported).

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@andrewbranch
Copy link
Member

If the user installs somelib, but they never import the somelib/FooBar.ts file (or somelib/FooBar.{js/d.ts} if the package is compiled), they will still have the definition of 'foo-bar': FooBar.

This isn’t true generally. When you import a package root we don’t crawl all the files/declarations in that package, we just go to the main/types and then recursively go through whatever that imports/references. So if the package index file imports or reëxports from FooBar.ts, then yeah, you get it transitively. But if it’s just sitting there alongside the index, it will never make it into the user’s program until they reference it themselves.

I guess the only places this proposal could have an effect is in files that were included for a reason other than a transitive reference from an root file, which would be lib, types/typeRoots (including the default of everything in @types), and include itself. I think doing this for lib files would be pretty obviously the opposite of what you want (the whole point of lib is to get global declarations that you don’t have to manually reference in your source files). If you don’t like what’s being included by default by types or typeRoots, you should probably tweak that setting in your tsconfig. And it seems very unlikely that this would be valuable on your own included files, but if you don’t like getting your own global declarations without an explicit reference, you should probably switch from include to files.

Am I totally misunderstanding you? It seems like what you’re proposing is already the way it works.

@andrewbranch andrewbranch added the Needs More Info The issue still hasn't been fully clarified label Dec 22, 2020
@trusktr
Copy link
Contributor Author

trusktr commented Jan 18, 2021

@andrewbranch I think that may be where my confusion comes from: that a library has types in package.json, but I was importing specific files from within it to try to avoid bringing in globals (the files I was importing do not import all files like the index file pointed to by types does).

I have to experiment a bit more to reproduce the behavior I saw, this time paying attention to types in package.json.

@trusktr
Copy link
Contributor Author

trusktr commented Jan 18, 2021

On a similar note, I had

types: [],
typeRoots: []

in tsconfig.json, and I was not importing any files that indirectly imported files with declare global, but TS still picked up globals anyway. I'm assuming this is because of the types behavior you described.

If I understand correctly, based on what you said, I can import one specific file from a package (a file that does not import any global types, indirectly or not), but if the project has a types field that points to a file that does import globals, I will get those globals anyway.

If so, that's most likely the problem I have, and the reason for the issue, and empty types and typeRoots was not usable for a workaround.

@andrewbranch
Copy link
Member

If I understand correctly, based on what you said, I can import one specific file from a package (a file that does not import any global types, indirectly or not), but if the project has a types field that points to a file that does import globals, I will get those globals anyway.

No, if you import a specific file from a package, we only process that file and the things it imports. The types entrypoint plays no role in this scenario.

@trusktr
Copy link
Contributor Author

trusktr commented Jan 20, 2021

Alright, then I'll close this for now, as there is no need for an option. I must have overlooked something in my case.

@trusktr trusktr closed this as completed Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

2 participants