chevrotain icon indicating copy to clipboard operation
chevrotain copied to clipboard

Published 20 hours ago • verified publisher icon Chevrotain

Update generate.ts

Open kevinkhill opened this issue 2 years ago • 12 comments

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

kevinkhill avatar Mar 12 '22 16:03 kevinkhill

image

kevinkhill avatar Mar 12 '22 16:03 kevinkhill

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

kevinkhill avatar Mar 12 '22 19:03 kevinkhill

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

bd82 avatar Mar 13 '22 01:03 bd82

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 avatar Mar 13 '22 02:03 bd82

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

  • https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/consistent-type-definitions.md

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

bd82 avatar Mar 13 '22 02:03 bd82

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

bd82 avatar Mar 13 '22 02:03 bd82

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.

NaridaL avatar Mar 13 '22 08:03 NaridaL

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 avatar Mar 13 '22 10:03 bd82

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

bd82 avatar Mar 22 '22 19:03 bd82

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

kevinkhill avatar Mar 22 '22 21:03 kevinkhill

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 avatar Mar 25 '22 17:03 kevinkhill

image

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

kevinkhill avatar Mar 26 '22 13:03 kevinkhill

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

bd82 avatar Jul 08 '23 23:07 bd82

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 🍻

kevinkhill avatar Jul 08 '23 23:07 kevinkhill