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

commitlint,docs: start using FP patterns #174

Merged
merged 10 commits into from
Sep 10, 2024
Merged

commitlint,docs: start using FP patterns #174

merged 10 commits into from
Sep 10, 2024

Conversation

knocte
Copy link
Member

@knocte knocte commented Sep 5, 2024

No description provided.

@knocte
Copy link
Member Author

knocte commented Sep 5, 2024

@webwarrior-ws hey Taras can you investigate why the 2nd commit of this PR breaks tests please? I'm very confused.

@webwarrior-ws
Copy link
Contributor

webwarrior-ws commented Sep 5, 2024

@knocte you've used class None instead of its instance, and that lead to instanceof None check to always fail.
I've added commit to fix it: https://github.com/webwarrior-ws/conventions/commits/wip/fp/

@knocte
Copy link
Member Author

knocte commented Sep 5, 2024

Thanks

And make use of it in one of the plugins already.

See my SO post about this:
https://stackoverflow.com/a/78937127/544947

Co-authored-by: webwarrior <[email protected]>
@knocte knocte force-pushed the wip/fp branch 4 times, most recently from 65ac82d to 00f6eef Compare September 6, 2024 03:28
Empty classes result in strange issues that turn the
TS compiler into not very helpful. E.g.:
https://stackoverflow.com/questions/78953267/why-is-typescript-not-throwing-a-compile-time-error-when-constructor-is-not-used

NOTE: we still mark the new methods as deprecated to
give a hint that `instanceof` usage is better. These
tooltips will be properly shown by IDEs and linters
according to:
https://stackoverflow.com/a/62991642/544947
@knocte knocte force-pushed the wip/fp branch 2 times, most recently from 05ef34b to ef4ddb9 Compare September 6, 2024 05:09
The keyword `instanceof` is a footgun in JS/TS because it
only works for classes, but not primitive types.

This was taken from:
https://stackoverflow.com/a/58184883/544947
@webwarrior-ws
Copy link
Contributor

There was a bug in IsInstanceOf, I fixed it - see last commit in https://github.com/webwarrior-ws/conventions/commits/wip/fp/
CI is red because of formatting in other files.

@knocte knocte force-pushed the wip/fp branch 4 times, most recently from 73e0062 to ef97226 Compare September 10, 2024 03:08
@knocte knocte force-pushed the wip/fp branch 3 times, most recently from 8576978 to bcffbc8 Compare September 10, 2024 06:00
@knocte knocte force-pushed the wip/fp branch 2 times, most recently from 6807c3d to 2100e2c Compare September 10, 2024 07:37
@knocte knocte changed the title wip/FP commitlint,docs: start using FP patterns Sep 10, 2024
@knocte knocte merged commit aed7a80 into master Sep 10, 2024
20 checks passed
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

Successfully merging this pull request may close these issues.

2 participants