Chevrotain / chevrotain

Parser Building Toolkit for JavaScript
https://chevrotain.io
Apache License 2.0
2.44k stars 200 forks source link

Update generate.ts #1782

Closed kevinkhill closed 1 year ago

kevinkhill commented 2 years ago

I read in the TypeScript docs that interface is preferred over type since they can be extended and merged. What do you think?

kevinkhill commented 2 years ago

image

kevinkhill commented 2 years ago

noooooooope, nevermind. I am naive. I don't know why type / interface breaks other parts of the code image

bd82 commented 2 years ago

I think the generated syntax was incorrect = sign should be removed when using interfaces

bd82 commented 2 years ago

I don't know why this eslint rule prefers interface but I don't think its because of extensibility reasons. Basically TypeScript uses a Structured type system as opposed to a Nominal type system (e.g Java). So everything is basically a set of properties and types. (If it walks like a duck and it quacks like a duck its a duck...)

See this Playground example an interface can even extend a class...

bd82 commented 2 years ago

After inspecting at the ESLint rule, it is not about one being better than the other, simply a question of project wide style consistency

The optimal workaround would likely be to exclude generated files from the ESLint inspections.

bd82 commented 2 years ago

Closing this PR as this seems like a subjective style question... 😄

NaridaL commented 2 years ago

I don't know why this eslint rule prefers interface but I don't think its because of extensibility reasons. Basically TypeScript uses a Structured type system as opposed to a Nominal type system (e.g Java). So everything is basically a set of properties and types. (If it walks like a duck and it quacks like a duck its a duck...)

See this Playground example an interface can even extend a class...

interfaces are extensible in the way that you can redeclare the interface somewhere else and the definitions will be merged. This feature is useful for generated code.

bd82 commented 2 years ago

interfaces are extensible in the way that you can redeclare the interface somewhere else and the definitions will be merged. This feature is useful for generated code.

👍 re-opening PR

bd82 commented 2 years ago

I'll try to review and promote this PR over the coming weekend

kevinkhill commented 2 years ago

I've been knee deep in chevrotain land with my project. It's been invaluable! I am happy to contribute however I can.

kevinkhill commented 2 years ago

You probably would have seen this too, but I just found another opportunity for type enrichment! Not only can the types be swapped to interface but you can also then have them extend CstChildrenDictionary 😊

image

becomes

image

kevinkhill commented 2 years ago

image

I tried cloning the repo and puttering with my tweaks and yeah, it's deeper than I expected 😬

bd82 commented 1 year ago

I've finally have some time to do some maintenance...

Anyhow I've looked into this: While its true you can auto-merge interfaces (with same name in TypeScript). I would argue that it is better to do: Interface X extends type Y {} when dealing with generated code.

That way if the generated type Y's name is changed a compile error message would appear and the user would have to deal with it.

TypeScript playground link

kevinkhill commented 1 year ago

Happy to hear you have time to work on this 😊 I know how hard it is to carve out time.

To be honest, I haven't touched the code that I was using before with this. I wish I could, it was fun, but life moves on.

Good luck with the library. I loved using it and will keep it as a tool in my belt.

Cheers 🍻