hybridsjs / hybrids

Extraordinary JavaScript UI framework with unique declarative and functional architecture
https://hybrids.js.org
MIT License
3.05k stars 85 forks source link

fix(types): Strict store types #246

Closed Qsppl closed 6 months ago

Qsppl commented 7 months ago

A more detailed description is in the commits themselves, but not in the PR.

codesandbox-ci[bot] commented 7 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

coveralls commented 7 months ago

Coverage Status

coverage: 99.955%. remained the same when pulling 2939aa65f83e7e23c2533e3367c40245620e78e5 on Qsppl:main into b78e7c40cba6517298ef8a61c6b1b59aa133666e on hybridsjs:main.

Qsppl commented 7 months ago

Probably need to add

.test.ts

to .npmignore

smalluban commented 7 months ago

Thanks for the PR. I'll look at the changes on my local machine soon.

Tests should be placed in /test/typescript/... - the repo structure separates them from the source. And /test/ is already ignored. Also, I think it would be great to add a Typescript compiler as a dev dependency and add an npm script to run tests on those files.

Qsppl commented 7 months ago

I'll do it today.

Qsppl commented 7 months ago

Notice that I replaced the "module declaration" in the types\index.d.ts file to a valid module for compatibility with ESM modules both within the package and for users of the package.

This shouldn't cause any problems, but just in case, check if the behavior has changed in project builders like "Vite".

Qsppl commented 7 months ago

I made several formatting errors in previous commits. In the Last Three Commits, I explicitly declared the project's formatting settings in the project so that other developers could immediately write code without having to configure their IDE. I also added recommendations for installing extensions for VSCode for technologies that it does not support natively.

Qsppl commented 7 months ago

I did everything? Can you accept PR?

smalluban commented 7 months ago

I'll review it today :)

smalluban commented 7 months ago

Generally, it looks fine. I am not yet convinced about adding some meta configuration for the editor things (but recently I listened to the talk that recommended it). As a weekend is ahead, I'll be back next week :) I hope that it's not a problem.

Qsppl commented 7 months ago

Have a nice weekend)

Qsppl commented 6 months ago

Hello, if in order to accept PR you need to do additional work or explain in more detail some changes in PR - write, I will do this work.

Qsppl commented 6 months ago

I don't know what the mistake is.

Safari 17.0 (Linux x86_64) router: test app connected navigate by pushing and pulling views from the stack FAILED
    RangeError: Maximum call stack size exceeded. thrown
    RangeError: Maximum call stack size exceeded. thrown
    RangeError: Maximum call stack size exceeded. thrown
    RangeError: Maximum call stack size exceeded. thrown
    RangeError: Maximum call stack size exceeded. thrown

This is it?

smalluban commented 6 months ago

Sorry, my fault. I forgot to add one missing fix with the latest commits. Pull main, it should be fine now.

smalluban commented 6 months ago

There was a bug there. Please rebase last time, and it will be ready to be merged.

Qsppl commented 6 months ago

All is ready?

smalluban commented 6 months ago

Yes, thanks for your contribution 🎉