Skip to content

TypeScript Should Allow for Suppressing Definition Errors Sourced from External Definition Files #8124

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
thoughtentity opened this issue Apr 17, 2016 · 4 comments
Assignees
Labels
Declined The issue was declined as something which matches the TypeScript vision External Relates to another program, environment, or user action which we cannot control. Suggestion An idea for TypeScript @types Relates to working with .d.ts files (declaration/definition files) from DefinitelyTyped

Comments

@thoughtentity
Copy link

Issues with bundled NPM definition files out in the wild with Microsoft/BotBuilder and angular/zone.js highlight how definition errors can impact down level TypeScript consumers of a package. TypeScript should provide an option of suppressing errors that are sourced from external definition files.

In microsoft/botframework-sdk#105, the bundled TypeScript definition will produce build errors if a down level consumer's tsconfig.json file has the noImplicitAny option set. In angular/zone.js#297, the bundled definition file isn't authored as an external module which will cause a build error during a build.

To fix the build issues raised in the referenced issues a down level consumer would either need to disable sourcing definition files from NPM packages, wait for the definition fix from package author or manually patch the definition file in the package's node_modules folder.

@billti billti added the @types Relates to working with .d.ts files (declaration/definition files) from DefinitelyTyped label Apr 18, 2016
@billti
Copy link
Member

billti commented Apr 18, 2016

Assigning to @RyanCavanaugh for his thoughts, as he is working on improving our type acquisition story.

@billti billti added this to the TypeScript 2.0 milestone Apr 18, 2016
@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Apr 19, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Apr 22, 2016

This seems like a problem with the definition file that needs to be addressed. your project dependency definitions contribute to the types in your project, and if you want the compiler to flag noImplicitAny in your project, it should not exclude some just because they originated in a .d.ts file. Practically, you can not be more strict than your libraries.

The solution here would be for Microsoft/BotBuilder to be built using noImplicitAny to ensure that their users can opt into a stricter type checking mode if they so choose. this is the standard for definitions on definitelytyped as well.

Suppressing errors coming from dependnecy definitions could result in missing legitimate issue, assumes dependency on error recovery behavior on the compiler, and eventually will result in unexpected behavior for library users.

@mhegazy mhegazy closed this as completed Apr 22, 2016
@mhegazy mhegazy added Declined The issue was declined as something which matches the TypeScript vision External Relates to another program, environment, or user action which we cannot control. and removed In Discussion Not yet reached consensus labels Apr 22, 2016
@thoughtentity
Copy link
Author

Point taken on suppressing external errors. Though I still think there should be some type of story around handling external definition errors. I think an argument can be made between your own code and external code that you don't explicitly control in terms of error handling. I don't think it's a good experience that bundled definition files picked up from npm can break the build. Especially, when it's over a minor thing such as no implicit any which shouldn't have any runtime implications.

Consider, providing an override mechanism so a down level consumer could provide their own definition file that overrides the npm package definition if error suppression is unpalatable. Right now, there is no way override definitions from npm packages if node module resolution is in play. A down level consumer would have to make an all or nothing choice if they want to take the bundled npm definitions. This would at least give down level consumers an out to unblock themselves without having to wait on the package author or manually patch the package's definition files in node_modules.

At the very least, standard guidance and best practices on how to author definitions so that they have the least impact on down level consumers should be provided to assist package authors.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 25, 2016

Consider, providing an override mechanism so a down level consumer could provide their own definition file that overrides the npm package definition if error suppression is unpalatable.

This should be available with TS 2.0 as part of #7156, we are doing a set of changes regarding how definition files are managed, included and acquired, see https://github.com/Microsoft/TypeScript/issues?utf8=%E2%9C%93&q=label%3ATypings+

At the very least, standard guidance and best practices on how to author definitions so that they have the least impact on down level consumers should be provided to assist package authors.

Yup. this has been rather scarce, i have to admit. different pieces are landing in place now, once that is done, the next step is providing a lot of documentation.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision External Relates to another program, environment, or user action which we cannot control. Suggestion An idea for TypeScript @types Relates to working with .d.ts files (declaration/definition files) from DefinitelyTyped
Projects
None yet
Development

No branches or pull requests

4 participants