symfony / webpack-encore

A simple but powerful API for processing & compiling assets built around Webpack
https://symfony.com/doc/current/frontend.html
MIT License
2.23k stars 197 forks source link

[RFC] Ship typescript types for the public APIs #816

Open stof opened 4 years ago

stof commented 4 years ago

Several IDEs (including PHPStorm/WebStorm and VS Code) are using the typescript type definitions to enhance their autocompletion for Javascript as well. While investigating #815, I made a small experiment: I generated a index.d.ts file in webpack-encore to have type definitions for the Encore public API (which is defined and documented in index.js). And this solved the type support for PHPStorm 2020.2.

Given that the type declaration can be fully generated from the existing JSDoc, I suggest that we bundle it in the package. For the record, here is the command I ran (at the root of the package): tsc --declaration --allowJs --emitDeclarationOnly index.js.

To make this production-ready, a few changes would be necessary:

All the improve JSDoc steps could actually be done even without shipping typescript type declarations, as they would still proper better type definitions. And actually, I have part of them done locally as part of my experiment.

What do you think @Lyrkan @weaverryan ?

My own vote would be in favor of including them, given that we don't need to maintain the .d.ts file manually.

stof commented 4 years ago

Note that even if the IDE decides to take the .d.ts file as the autocompletion source, all the explanations written in the JSDoc are preserved, as tsc will copy the JSDoc to the type declaration.

stof commented 3 years ago

@Lyrkan @weaverryan any opinion on this ?

chapterjason commented 3 years ago

@stof

Note that even if the IDE decides to take the .d.ts file as the autocompletion source, all the explanations written in the JSDoc are preserved, as tsc will copy the JSDoc to the type declaration.

You can also reference the .d.ts in the package.json Including declarations in your npm package

In long term I would suggest writing the source in typescript, couldn't find any considerations about that, but I don't think there are any drawbacks.

stof commented 3 years ago

You can also reference the .d.ts in the package.json Including declarations in your npm package

that's irrelevant for this comment. My comment does not care about how the d.ts file is discovered by the IDE.

In long term I would suggest writing the source in typescript, couldn't find any considerations about that, but I don't think there are any drawbacks.

that's a totally different topic. And given that Encore deals with tons of optional dependencies, it might be hard to make it typed (without using any in lots of places, defeating the purpose). For now, I'm not convinced of the benefits of rewriting Encore to Typescript. But I am convinced of the benefits of better code completion in IDEs (btw, it will also help people writing the webpack config file in typescript). In any case, that's of-topic for this RFC.

Lyrkan commented 3 years ago

@Lyrkan @weaverryan any opinion on this ?

That would be a great thing to have. We will also have to update CI jobs in order to check that PRs don't break the generation of that file.

In long term I would suggest writing the source in typescript, couldn't find any considerations about that, but I don't think there are any drawbacks.

I kinda agree with stof regarding that point, it would probably require a lot of work for not that many benefits due to all the dependencies we work with (and all their supported versions). The only thing we really need is the public API to be typed correctly and recognized by IDEs.

Also there is another small drawback: currently people can use an unreleased version of Encore quite easily by referencing the git repository in their package.json (it can be quite useful, for instance, to ask people if a fix works as intended). Switching to Typescript would add some extra steps to that process and/or require to automate some kind of "experimental" releases on npm.

TavoNiievez commented 3 years ago

This would make using a webpack.config.ts to configure Encore offer many benefits over the contextual help that the IDE can provide. I think this is a good proposal.