knockout / knockout

Knockout makes it easier to create rich, responsive UIs with JavaScript
http://knockoutjs.com/
Other
10.43k stars 1.52k forks source link

@types/knockout npm package declared as ambient module instead of exporting namespace #2588

Open andy1547 opened 2 years ago

andy1547 commented 2 years ago

Firstly I'd like to thank all the contributors for creating such a fantastic project, it's really stood the test of time & has been paramount in the construction of many rich UIs, you've done yourselves proud.

Currently the @types package declares an ambient module "knockout" before exporting with export = (exports a single object in CommonJS/AMD format). I'm not sure if the ambient module part is required / recommended, so I've carried out some research on modern declaration authoring practices.

For advice relating to choosing a UMD template, see https://www.typescriptlang.org/docs/handbook/declaration-files/templates/global-plugin-d-ts.html#examples-of-umd-libraries. Looking at the v3.5.1 .js dist, I believe we need to expose a object so the module template provided here is most appropriate https://www.typescriptlang.org/docs/handbook/declaration-files/templates/module-d-ts.html#default-exports.

There appears to be a choice when authoring whether to use the new ESM default export syntax with esModuleInterop:true or export =, I would have thought the latter is the best choice for backwards compatibility.

My question why does the current definition @types/knockout/index.d.ts v3.4.71 define an ambient module? Can the export as namespace myLib syntax be used to handle global usage (when used without a module loader)? If you'd like some examples look at chart.js/jquery/moment.

Secondly I noticed the official npm knockout package includes types but defines them rather differently, which package should consumers use going forward?

Thanks in advance!

webketje commented 2 years ago

Knockout's dist already ships with a UMD wrapper, and already exports an object, see cdnjs debug build

@types/<anything> is usually added before <anything> ships with official types within its package and maintained by unofficial contributors. So it is likely that the types shipped with knockout npm package are more up-to-date. But don't take my word for it, you can probably verify it by looking at the commit dates per file in the @types repo vs the types file here.

Not 100% sure, but it looks to me like the wanted config isesModuleInterop: false, but definitely allowSyntheticDefaultImports: true