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

Discussion: Should webpack-cli use TypeScript? #245

Closed
evenstensberg opened this issue Jan 20, 2018 · 31 comments · Fixed by #550
Closed

Discussion: Should webpack-cli use TypeScript? #245

evenstensberg opened this issue Jan 20, 2018 · 31 comments · Fixed by #550

Comments

@evenstensberg
Copy link
Member

evenstensberg commented Jan 20, 2018

Webpack-CLI is responsible for other projects within webpack such as webpack-dev-server and webpack itself. The Dev Server had some problems using babel in their projects if babel was included in the parent folder inside node_modules. We patched this in a new minor of the CLI, but we were using babel for outputting some tips in migrate, as well as for our workflow with flow.

As we cannot use Flow anymore, there's a discussion about introducing TypeScript, which might or might not be a good idea. The CLI should be well documented, with strong types as well as it should be easy to contribute to the CLI. With Type Systems, it might be increasingly harder to contribute to webpack-cli, and in the light of this, we might not have any type system at all.

Why have a Type Checker anyway?

We used Flow earlier to make sure that the AST transforms and its respective documentation was solid, meaning that it should be fairly easy to know exactly what every function that touches any form of AST would do. Therefore, we decided to gradually use Flow in some parts of the source code. As complexity arise, type systems are golden, not only for developers to know exactly what types that goes into the function, but also how to manipulate those types to do what we want them to do.

However..

This, again, makes it hard for external contributors to start helping out. Our goal, is to make contribution as easy as possible. Introducing TypeScript or Flow, might make it harder to submit a Pull Request to the repository, and that is not the superlatives we want.

What could we do?

One thing we could do, is to write more documentation to the source code, but this falls within the scope of over-documenting everything. The source code again becomes messy, and it's a possible pit we can fall into.

The babel issue could also be more investigated, and we'd reintroduce Flow. Reintroducing Flow would mean that it would be harder to contribute, because the essential question we're asking, is "How to make the CLI better to maintain and to contribute to"

What would you as a new contributor prefer?

@iamclaytonray
Copy link

iamclaytonray commented Jan 20, 2018

I personally prefer (love) using TypeScript. I also think it's a lot easier to contribute to a TypeScript codebase. I could go on but I'd really just be listing off general benefits from TS.

I'd also be completely supportive of the move over to TypeScript and send PRs if that decision were made.

@OrenMe
Copy link

OrenMe commented Jan 20, 2018

As we cannot use Flow anymore

Why can’t flow be used?

@evenstensberg
Copy link
Member Author

We had an issue that were if we used babel in the CLI, it would be as a parent in node_modules and broke webpack-dev-server. Pinging @shellscape for a more precise explaination

@mikehaas763
Copy link

mikehaas763 commented Jan 21, 2018

It would definitely lower the barrier to entry for me if it was TypeScript. Type systems can be intimidating to someone who has had no prior exposure to them, but I think it still actually decreases the barrier to entry for them as well.

Take a guess at the types of the parameters here:

function doSomething(things, errorCode) {}

I would have guessed an Array for things, not a K,V Map, and would have been unsure whether errorCode was a number or a string. I'd then find myself digging into the implementation of doSomething when I really don't care.

A typed version:

function doSomething(things: Map<string, string>, errorCode: string) {}

This is a lot more clear to me, and with either a well named self documenting function name or a small comment description, I'd be able to infer exactly what sort of values I should pass in for the specific types.

@itsMapleLeaf
Copy link
Contributor

If we do move over to TS, I'd be happy to help with the conversion 💯

@mikehaas763
Copy link

Same. I have experience doing full TypeScript conversions, and have done it incrementally in the past. Radical idea: to take advantage of the webpack ecosystem, I now use it for node.js programs as well, let's use webpack to build the webpack-cli! 🙂

@itsMapleLeaf
Copy link
Contributor

Using webpack to build it might be overkill 😛 A simple tsc --watch setup would work fine I think

@shellscape
Copy link

(I can't wait to see how many downvotes this gets) I'm not in favor introducing any kind of type checking in JavaScript. If I wanted to use a typed language, I'd of stuck with .NET 😄

@ematipico
Copy link
Contributor

I used TypeScript in the past and I started the introduction of Flow inside the repository. A typed language is surely something valuable and brings benefits. Although typings require more typing and sometimes it's difficult to following the streaming of methods and typings inside the different functions. I talk for personal experience on this repo, sometimes it was difficult to understand which type to return inside a function (in order to not break the rest).

I am still skeptical about why babel was a problem for dev-server, I think there is something fishy here. Regardless, when I worked with TypeScript I had to stick babel under the hood because the transpiling system is a joke, really.

TypeScript
Babel

This was one of the reasons why I chose to have a babel loader inside my code, the TypeScript transpilation is not great at all.

I'd prefer to stick with plain JavaScript, personally

@itsMapleLeaf
Copy link
Contributor

itsMapleLeaf commented Jan 22, 2018

@ematipico TS' transpilation might not be as robust or aggressively spec-compliant as babel's, but it still does the job well.

I'd also like to point out that both of those code examples are erroneous. It should be of instead of in, and in TS, the left side of a for..of expression can't (and shouldn't) be annotated, so it should just be for (const [key, value] of theMap) {. I'll also note that, for the TS example to be compiled properly, either the target must be set to es6, or the "downlevelIteration" option enabled in the tsconfig, which isn't available in the online playground. Here's an example with es5 and downlevelIteration.

Either way, I'm open to adopting Flow as well if the problems with it ever get addressed, if the babel ecosystem is important to the project. Static types are a huge plus for maintainability.

@ematipico
Copy link
Contributor

Yeah I made a mistake, my intention was to stick the for...of loop, which is transplited in a simple for loop with a check of . This was the reason why I had to add babel to the transpilation process. But I talk for my personal experience and every project is different. @shellscape is it possible to have more details about the impossibility of having babel as devDep inside the project?

@fokusferit
Copy link
Contributor

@mikehaas763 @kingdaro but isn't typescript more leveling the barrier up instead lowering it? Even if it looks almost like plain JS there might be some different aspects.
And from understanding code, well, sticking to clear naming, clean code patterns + initial JSDocs comments would already solve a lot of readability/understanding problems.

I'm pro type system as they reduce bugs/issues especially when a lot of data flows between different functions but I think the project might need to fix other issues first before we start refactoring it.

And yes, I really want to understand what the exact problem with webpack-dev-server and babel/flow was, maybe fixing that problem already solves this one :)

@iamclaytonray
Copy link

@fokusferit - You can still do JSDocs throughout the codebase. In fact, I do within my TypeScript and JavaScript. But documenting APIs and typing them are different. Reading docs vs compile-time errors can drastically improve code that is shipped. Clear naming doesn't verify what arguments a function should receive as well as what type those arguments should be. (Look at the example from @mikehaas763 above.)

But types aren't the only thing TS provides. Enums, typings, interfaces, etc. And if this codebase is written in TypeScript, there is no reason to have declarations on @types/. (In case anyone uses TS in a project and uses webpack-cli within said project)

@iamclaytonray
Copy link

^ I should add, documenting APIs should happen regardless of using a typed language or not. But we all know this 😄

@shellscape
Copy link

I love getting emails, but edit buttons are nifty.

@elevenpassin
Copy link
Contributor

elevenpassin commented Feb 7, 2018

A type system like typescript would be great!

@iamclaytonray
Copy link

I'd like to add:

For developers who work with statically typed languages, moving over to TS may encourage them to also contribute to webpack-cli. I honestly think statically typed codebases are a lot easier to contribute to. Of course, other people may disagree. I digress. I would hate to see a bunch of comments throughout the source covering types, which I believe would be the only workaround with using Flow. I could be wrong though. It looks like we are on a 12 - 0 streak so far 😄

A simple example but it gets the point across:

Several months ago, Graphcool's templates were written in JavaScript. When they decided to rewrite everything in TypeScript, there were a lot of people who didn't really know TS but thought it was a good opportunity to learn it. I'm not saying that would happen here but I think it is definitely possible.

@bitpshr
Copy link
Member

bitpshr commented Mar 16, 2018

Huge +1 for TypeScript when it comes to building and maintaining a widely-used tool, we wrote Dojo 2 entirely with it. The simple ability to modify a function signature, breaking it's previous API, and to then have all uses of that function immediately fail to compile until fixed, is powerful. I can't think of any reason not to use it other then not introducing additional learning curve for those that haven't used it before.

  • It greatly reduces the pain of API changes.
  • It can be added iteratively and not all at once.
  • Similarly, types can be strengthened over time.
  • Easier doc tools (TSDoc, no type annotations required.)

I'd be happy to help with this @ev1stensberg, just let me know.

@cspotcode
Copy link

cspotcode commented Mar 22, 2018

Another +1 for TypeScript.

IMO avoiding a type system makes all contributions more difficult because it's tougher to learn the codebase and make changes that don't break anything. Type information tooltips, documentation popups, and code-completion are big productivity boosts. Run git clone, then npm install, and the language service automatically boots in your editor.

On some of my projects, I've accepted PRs with incomplete type annotations, and I just fix the annotations before merging. If the logic is correct, this step isn't difficult.

I've never had any issues with TSC's transpilation; it's usually a bit simpler than babel's. Or you can use babel's new TS support if you prefer and run tsc --noEmit on CI to check for type warnings.

recast even support TS now, so you might be able to codemod at least some of the conversion.

@ev1stensberg happy to help and answer specific questions as well.

@evenstensberg
Copy link
Member Author

@cspotcode thanks. Think this sways majorly in favor for TypeScript, so we're going to implement that.

@ematipico
Copy link
Contributor

We started a draft about a graceful implementation of TypeScript together with @dhruvdutt
We can takeover what was done with Flow, especially the interfaces, adapt it to TypeScript and from there beginning a transition

@scoobster17
Copy link

On a side note, could in the same way we get options whilst running init of Sass/Less etc for CSS, could we also get options to use TypeScript instead of Babel? And I've never used it, but perhaps Flow as it has been mentioned here before.

I assume this will feed into the decision as to whether to switch or not?

@alexander-akait
Copy link
Member

I believe that rewriting the project on typescript is overhead. Why? Because we have a lot of bugs/feature requests in all webpack ecosystem, also we have webpack-command (https://github.com/webpack-contrib/webpack-command) (official second CLI), he is faster, plugable and modular and IMHO has better structure and easier to maintain. What will the rewriting on typescript for webpack-cli? I do not see at the moment any advantages other than typing, but why do we need typing here?

For me it's a waste of time. People have already started migration on webpack-command and while the project will rewriting (i.e. no features and maybe also bug fixes). The project can simply become abandoned.

I propose to discuss on future CLI for webpack@5 together (for me plugable and modular webpack-command is more better for maintenance). And do not make a rash decision. Thanks!

@alexander-akait
Copy link
Member

/cc @ev1stensberg friendly ping, what do you think about my post above?

@evenstensberg
Copy link
Member Author

It makes sense, and I agree. Maybe the reason why I'd favor TypeScript is that modules are easier to work with and to make sense of. We can have it partially as we've got mono-repos now. Webpack-cli itself wouldn't have typescript, but init, migrate et al would.

@alexander-akait
Copy link
Member

@ev1stensberg and what do you think about webpack-cli and webpack-command? Perhaps we should unite efforts over one CLI (webpack-command is plugable). init and migrate just additional package instead hardcode in code base.

@evenstensberg
Copy link
Member Author

Trying to work on that this summer. Been working on the plugable part and it's near end. One point I mentioned to Andrew was that we can detect if the user uses webpack for the first time and gradually make the CLI for them more complex over time. I.e usage of more advanced features are available as they use the tool.

Regardless, doing that later this summer.

@sendilkumarn
Copy link
Member

Perhaps we should unite efforts over one CLI (webpack-command is plugable)

I was planning to raise an issue on webpack-command on this. I think we should discuss on this together on a meeting.

For typescript migration

I am all in favor of this but since we are also doing webpack-command it is good that we have to unite our efforts.

@fokusferit
Copy link
Contributor

Well, I think in such case it is the best to come together as @sendilkumarn mentions and really have a meeting around the "purpose" what we as developers expect from webpac-cli/webpack-command . Based on an agreed purpose, this would create a goal we could break down into a lot of smaller topics and work on together.

@evenstensberg
Copy link
Member Author

Also worth mentioning webpack-cli and command have two different solutions w.r.t what we're trying to do as per now. The CLI is/was focusing on ecosystem previously, command is on beginner friendliness. We're going to tackle that part soon, but this is OSS and you can't devote 8h a day to do everything people expect in a fingersnap.

We're getting there, but it takes time to build a strong tool. Andrew did a great job with command and I'd like to make sure that we don't waste time doing the same thing if that CLI is working well for beginners.

@evenstensberg
Copy link
Member Author

Closed 🎉 Our main cli folder will remain in js as it would be easier for beginners to make sense of and then move on to understand each package. Cheers & thanks to our GSOC student @dhruvdutt !

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

Successfully merging a pull request may close this issue.