solid js library #37

matthewgapp · 2024-01-01T23:32:26Z

Port of the React library for Soild JS. Have it working with a downstream demo app, which is also a port of the React SQLSync demo.

Todo

Fix FE tests
- The "sanity" test is copy pasta from the react directory but didn't take the time to build and run it. I do however have a downstream app (solid js port of the todo demo) consuming the solid js library so I can confirm that it appears to be working fine.
Clean up impl and exports. Some botched implementation garbage hanging around from when I was debugging a build issue.

matthewgapp · 2024-01-01T23:35:13Z

.vscode/settings.json

@@ -22,5 +22,19 @@
    "editor.defaultFormatter": "biomejs.biome",
    "editor.tabSize": 2,
    "editor.insertSpaces": true
+  },
+  "workbench.colorCustomizations": {
+    "[Monokai Pro]": {


not sure where these came from lol

matthewgapp · 2024-01-01T23:35:54Z

lib/sqlsync-solid-js/src/sqlsync.ts

This could be a shared file or library between the Solid and React implementations.

matthewgapp · 2024-01-01T23:36:12Z

lib/sqlsync-solid-js/src/sql.ts

This could be part of the shared JS library

matthewgapp · 2024-01-01T23:36:26Z

lib/sqlsync-solid-js/src/util.ts

This could also be part of the shared JS library

carlsverre · 2024-01-01T23:39:35Z

Wow Matt! This looks awesome. I agree, I think that a ton of code needs to be refactored out into a shared js lib for the different frameworks. I didn't realize how similar the react and solid implementations would be - that's good to know! Appreciate the work here, will get back to this PR asap once I have a chance to play around with it and think about how I want to refactor things.

matthewgapp · 2024-01-01T23:42:15Z

Wow Matt! This looks awesome. I agree, I think that a ton of code needs to be refactored out into a shared js lib for the different frameworks. I didn't realize how similar the react and solid implementations would be - that's good to know! Appreciate the work here, will get back to this PR asap once I have a chance to play around with it and think about how I want to refactor things.

Thanks! Yeah, Solid JS and React APIs are very similar. I have a bit of cleanup to do and will mark as ready for review. I should get around to it this afternoon.

matthewgapp · 2024-01-02T01:20:59Z

Wow Matt! This looks awesome. I agree, I think that a ton of code needs to be refactored out into a shared js lib for the different frameworks. I didn't realize how similar the react and solid implementations would be - that's good to know! Appreciate the work here, will get back to this PR asap once I have a chance to play around with it and think about how I want to refactor things.

Thanks! Yeah, Solid JS and React APIs are very similar. I have a bit of cleanup to do and will mark as ready for review. I should get around to it this afternoon.

@carlsverre should be ready for a review. As noted in the description, I didn't run the test locally. Not sure how I should go about building and running the sanity test without investing more effort. I can confirm that it's woking in my downstream demo application. I'm about to fold it into my production application so will let you know if I run into any issues there.

matthewgapp · 2024-01-02T01:22:40Z

As an aside, I'm excited to use and contribute to this project. The first two items on my mind are pluggable auth and storage but I don't know the specifics around what I need just yet. I should know more soon as I start to integrate it into my application.

carlsverre · 2024-01-02T18:38:28Z

@matthewgapp No worries on testing, I'll do that as part of integrating this.

One thought I had is whether merging all the framework libraries into one would be better? With modern JS build systems, the unused library code should be ignored pretty well. That way I just have different imports for each framework but it would be easier to share code that's similar.

The other option is what you said above, which would be to refactor out the core SQLSync client class into a standalone library that each framework library depends on. I think that's probably the right approach, but curious what you think.

matthewgapp · 2024-01-02T19:09:45Z

@matthewgapp No worries on testing, I'll do that as part of integrating this.

One thought I had is whether merging all the framework libraries into one would be better? With modern JS build systems, the unused library code should be ignored pretty well. That way I just have different imports for each framework but it would be easier to share code that's similar.

The other option is what you said above, which would be to refactor out the core SQLSync client class into a standalone library that each framework library depends on. I think that's probably the right approach, but curious what you think.

I think it would depend on what your JS users would expect. I'm no expert on managing the all-so-tiring JS ecosystem but I'd err on the side of simplicity, both for authors and users. I think a thick JS core library and thin framework-specific wrapper approach aligns best against the goal of keeping things simple on the user side IMO. I'm not sure if a single library would be easier or worse for authors. However, separating the JS core from the framework libraries does ensure that the wrappers stay thin and decoupled, helping new framework authors get started without extensive knowledge of the JS core lib.

carlsverre · 2024-01-05T23:07:37Z

@matthewgapp I just landed the react refactor. Can you rebase this on the main branch? For some reason the current git state of this branch also includes all my changes.

carlsverre · 2024-01-05T23:08:31Z

also for testing, can you make a solidjs version of guestbook-react? you can also just re-use the existing reducer that's also in the examples directory.

…o queries have been tested

matthewgapp · 2024-01-06T06:08:06Z

also for testing, can you make a solidjs version of guestbook-react? you can also just re-use the existing reducer that's also in the examples directory.

@carlsverre implemented the guestbook and seems to be working. I noticed that you moved away from vite and to rollup for react library. How come? And would you like the solid js moved over to rollup too?

carlsverre · 2024-01-06T16:34:42Z

It's not strictly needed, but I moved over for consistency. Since I moved the example out of sqlsync-react and into examples, it no longer seemed that vite was as needed. Rollup is slightly easier to control for libraries whereas vite seems better when you are shipping web apps.

examples/guestbook-solid-js/src/main.tsx

carlsverre · 2024-01-06T16:52:17Z

lib/sqlsync-solid-js/src/context.tsx

+};
+
+export const SQLSyncProvider: ParentComponent<Props> = (props) => {
+  const [sqlSync, setSQLSync] = createSignal<SQLSync>(createSqlSync(props));


I'm not super familiar with how SolidJS works, but it appears that every time this component (SQLSyncProvider) renders it will re-create the SQLSync object. This is undesirable as each instance of the SQLSync object will initialize a connection to the shared worker at construction time. Is it possible to only call createSqlSync once if the signal has not yet been set?

Solid JS calls component functions once, so this should correctly create the SQLSync object once.

See here for more info: https://www.solidjs.com/guides/getting-started#2-vanishing-components

lib/sqlsync-solid-js/src/hooks.ts

carlsverre · 2024-01-06T16:55:47Z

lib/sqlsync-solid-js/src/hooks.ts

+  const [state, setState] = createSignal<QueryState<R>>({ state: "pending" });
+
+  createEffect(() => {
+    let query = normalizeQuery(rawQuery());


This can be a const. Also can you run biome on this PR? I'll add it to just soon, but in the meantime you can run:

pnpm exec biome check ./lib/sqlsync-solid-js/src

"let" haha, i've written too much rust. Ran biome. Thanks for the tip. Would you mind adding instructions on biome to the CONTRIBUTING.md?

matthewgapp · 2024-01-07T07:58:15Z

@carlsverre think i've addressed your comments. I moved the build from vite to rollup. Also fixed formatting but might not have caught everything. CI seems to be failing on unrelated LLVM stuff.

carlsverre

This looks awesome, gonna land it today. Thank you!

matthewgapp changed the title ~~started adding solid js library~~ solid js library Jan 1, 2024

matthewgapp commented Jan 1, 2024

View reviewed changes

lib/sqlsync-solid-js/src/sql.ts Outdated

Copy link

Contributor Author

matthewgapp Jan 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be part of the shared JS library

matthewgapp commented Jan 1, 2024

View reviewed changes

lib/sqlsync-solid-js/src/util.ts Outdated

Copy link

Contributor Author

matthewgapp Jan 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also be part of the shared JS library

matthewgapp marked this pull request as ready for review January 2, 2024 01:17

carlsverre mentioned this pull request Jan 4, 2024

Refactor sqlsync react #38

Merged

matthewgapp force-pushed the matt/feat/solid-js-library branch from 549144b to 861ca94 Compare January 5, 2024 00:20

matthewgapp added 13 commits January 5, 2024 15:13

started adding solid js library

222b806

more changes. just have to fix the test

8eb2188

wip: trying to get the library to work for host

41cbad6

downstream application is consuming solid library properly although n…

a469fe1

…o queries have been tested

some cleanup

08bd745

wip: more cleanup

22c28c0

wip: cleanup tests

f97af12

wip: remove vscode settings

19e9ecc

wip

b3751d6

wip

b87b359

fix

3c2b58d

cleanup after rebasing onto reeact refactor

44f4128

more cleanups and fixes

a72e0c5

matthewgapp force-pushed the matt/feat/solid-js-library branch from 7ed121c to a72e0c5 Compare January 5, 2024 23:14

matthewgapp added 3 commits January 5, 2024 19:50

add solid js guestbook

d9555b6

revert react changes

c336448

wip: remove reducer

0333e03

carlsverre reviewed Jan 6, 2024

View reviewed changes

examples/guestbook-solid-js/src/main.tsx Outdated Show resolved Hide resolved

carlsverre reviewed Jan 6, 2024

View reviewed changes

lib/sqlsync-solid-js/src/hooks.ts Outdated Show resolved Hide resolved

carlsverre reviewed Jan 6, 2024

View reviewed changes

matthewgapp added 6 commits January 6, 2024 23:34

move from vite to rollup

3d18fbd

fix exports

4646235

wip: more fixes

29ee7e0

wip: formatting

12e51c3

wip: formatting

83eeb8d

wip: update lockfile

65c103d

carlsverre reviewed Jan 7, 2024

View reviewed changes

carlsverre merged commit 518bae5 into orbitinghail:main Jan 8, 2024
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

solid js library #37

solid js library #37

matthewgapp commented Jan 1, 2024 •

edited

Loading

matthewgapp Jan 1, 2024

matthewgapp Jan 1, 2024

matthewgapp Jan 1, 2024

matthewgapp Jan 1, 2024

carlsverre commented Jan 1, 2024

matthewgapp commented Jan 1, 2024

matthewgapp commented Jan 2, 2024

matthewgapp commented Jan 2, 2024

carlsverre commented Jan 2, 2024

matthewgapp commented Jan 2, 2024 •

edited

Loading

carlsverre commented Jan 5, 2024

carlsverre commented Jan 5, 2024

matthewgapp commented Jan 6, 2024

carlsverre commented Jan 6, 2024

carlsverre Jan 6, 2024

matthewgapp Jan 7, 2024

carlsverre Jan 6, 2024

matthewgapp Jan 7, 2024

matthewgapp commented Jan 7, 2024

carlsverre left a comment

solid js library #37

solid js library #37

Conversation

matthewgapp commented Jan 1, 2024 • edited Loading

Todo

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlsverre commented Jan 1, 2024

matthewgapp commented Jan 1, 2024

matthewgapp commented Jan 2, 2024

matthewgapp commented Jan 2, 2024

carlsverre commented Jan 2, 2024

matthewgapp commented Jan 2, 2024 • edited Loading

carlsverre commented Jan 5, 2024

carlsverre commented Jan 5, 2024

matthewgapp commented Jan 6, 2024

carlsverre commented Jan 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewgapp commented Jan 7, 2024

carlsverre left a comment

Choose a reason for hiding this comment

matthewgapp commented Jan 1, 2024 •

edited

Loading

matthewgapp commented Jan 2, 2024 •

edited

Loading