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

Fix reference to deprecated handlebars type #1022

Merged
merged 3 commits into from
Apr 24, 2019

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented Apr 23, 2019

from handlebars.d.ts:

// NOTE: for backward compatibility of this typing
type HandlebarsTemplateDelegate<T = any> = Handlebars.TemplateDelegate<T>;

turns out it's not actually backwards compatible because they haven't declared it as an ambient global type of subtle differences in type acquisition (see below). but anyway, typedoc should use the new namespaced type. I've gone ahead and done that in this PR.

unblocks palantir/documentalist#86
cc @styu


edit: more info

Another way to solve this would be with an import "handlebars"; statement in any typedoc module which references a global handlebars symbol like HandlebarsTemplateDelegate. But adding import statements just for type acquisition side effects is bad.

When handlebars switched to publishing its own types, they did a straight port of @types/handlebars. However, the semantics for acquiring types from node_modules/@types/handlebars is a little different from node_modules/handlebars. TypeScript will automatically pull in all ambient types from node_modules/@types and make symbols like HandlebarsTemplateDelegate available, but the same is not true for types bundled in a regular npm package.

Related to this (but external to this repo), there's no need for declare module statements in handlebars' types anymore. The type system will acquire types from the same location as the module system, so declare module "handlebars" is unnecessary.

@adidahiya adidahiya marked this pull request as ready for review April 23, 2019 18:51
@aciccarello aciccarello merged commit cde478a into TypeStrong:master Apr 24, 2019
@aciccarello
Copy link
Collaborator

Great! Thanks for the PR!

@adidahiya adidahiya deleted the patch-1 branch April 24, 2019 14:50
@raymondfeng
Copy link
Contributor

@aciccarello Would you please publish a new release to npm?

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

Successfully merging this pull request may close these issues.

3 participants