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

Built-in type definitions / JSDoc #240

Open
scripthunter7 opened this issue Jan 30, 2023 · 2 comments
Open

Built-in type definitions / JSDoc #240

scripthunter7 opened this issue Jan 30, 2023 · 2 comments

Comments

@scripthunter7
Copy link
Contributor

scripthunter7 commented Jan 30, 2023

The code is under-documented, it would be worth considering at least adding JSDoc. I started combining JSDoc with type definitions, here is a sample file for the doubly linked list: https://github.com/scripthunter7/csstree/blob/feature/dts/lib/utils/List.d.ts

I have already created all the files for utils and tokenizer: https://github.com/scripthunter7/csstree/tree/feature/dts

The question is whether anyone can / wants to help with this?

If the code is documented, it's easier to contribute, and I think TypeScript type definitions are essential these days. The DefinietlyTyped file is quite incomplete, e.g. it does not contain the tokenizer or the fork.

I know that there is also a TypeScript / dts thread, but looking at the code, it is impossible to port it to TypeScript without breaking changes:
#88

@lahmatiy
Copy link
Member

lahmatiy commented Feb 2, 2023

I tried to rewrite CSSTree in TypeScript a couple of times. Add typing for List and other utils is quite a trivial problem that does not cause difficulties. However, a complex parts, like parser, are very non-TypeScript friendly due to their flexibility. At the moment, there is a choice, either typings or flexibility. Flexibility means the posibility of fork() method and the ability to provide specialized entry points (like css-tree/selector-parser).

The fork() method allows create an altered instance of API. For instance, you can extend parser, lexer and generator to work with new types of AST nodes, or alter parsing/serialization rules. This feature is used by other libs to add new syntax support or some kind of support for preprocessor's syntax (for instance in stylelint-validator - Saas and Less).

I'm still thinking about how to add TS typing and keep it flexible. For now, flexibility is more important than typing or performance.

@scripthunter7
Copy link
Contributor Author

scripthunter7 commented Feb 2, 2023

Add typing for List and other utils is quite a trivial problem that does not cause difficulties. However, a complex parts, like parser, are very non-TypeScript friendly due to their flexibility

I can confirm this 😅

Another main topic of my issue is JSDoc. Unfortunately, the code is documented in very few places. For example, there is a lot of "this" that needs to be debugged to see where it comes from through which binding. For example, when creating types, I should know what the basic code does, but as I reach more and more "flexible" codes, this becomes an increasingly difficult task.

The use of "this" also occurs during fork implementation, although I could deduce what it is based on the core code.

At the moment I was "only" talking about documentation and .d.ts files. In my opinion, full TS porting is only possible by breaking change. I think flexibility can be maintained in TS as well, but in a completely different way.

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

No branches or pull requests

2 participants