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

[FIX] Update start package #660

Merged

Conversation

eagerestwolf
Copy link
Contributor

@eagerestwolf eagerestwolf commented Jul 14, 2024

So, as I had mentioned in the Solid Discord server, I got the @solid-community/start package updated to work with Solid Start version 1 (and didn't even need to include @solidjs/start as a dependency).

THIS PULL REQUEST IS NOT READY TO MERGE

I only opened a pull request, so I could collaborate with the community/team about where to go from here. Ideally, I should probably create a provider and hook for a context, much how it's done with Kobalte UI.

There are a few notable changes so far:

  • Obviously solid-start is no longer a dev or peer dependency (for obvious reasons, it's unmaintained)
  • The current code is actually framework agnostic, it will work in any SSR/CSR Solid application (probably Vike as well, but I haven't tested that)
  • A ton of dependencies got removed from pnpn-lock.yaml, likely because there was a lot of duplication from the old versions of solidjs and solid-start floating around
  • As it stands, this implementation is 100% compatible with the existing implementation (I just brought it up to date)
  • Obviously, I had to insert my own implementation of a cookie parser. So, as not to bring in a useless package, I used the implementation that Kobalte uses, which was lifted from Chakra UI.

What's next?

Well, I need input from the maintainers (side note: I did not bump the version in package.json yet). What do you guys think the next step here. My opinion is that a context is the next step; however, I know this repository focuses on primitives and leaves implementation and expansion to the user. Therefore, I leave it in your guy's hands to determine the course of action. Sadly, this package is no longer usable for it's intended purpose in its current state since Solid Start now doesn't have the old Root.tsx, and anything declared in entry-server.tsx is non-reactive. Therefore, a context would be the only way to use this with Solid Start.

Copy link

changeset-bot bot commented Jul 14, 2024

⚠️ No Changeset found

Latest commit: a04bb61

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@thetarnav
Copy link
Member

thetarnav commented Sep 8, 2024

Thank you for looking into this.

Since start was removed from dependencies, I don't think it makes sense to keep the utilities in a start package. The package could be temporarily removed, to be reused if needed. (Although I don't really like the idea of a start package in general, why not try to contribute these utilities directly to start instead of making another library, right?)

The primitives could be moved to different packages, where it makes sense.
How about a cookie package, since the only utilities here are focused on that?
I know there is storage which handles cookies a bit, but I really don't want to put more things there, as it's already too big. I would enjoy a separate, smaller, simpler package way more.

@atk @Tommypop2 What do you think?

Also there is this: #449
Which adds one more cookie-related helper.

@davedbase
Copy link
Member

It's highly unlikely that these would be accepted into SolidStart. Our goal is to keep it very unopinionated.

This would be accepted into MediaKit though...

Some of these primitives work outside Start so moving them to the relevant Primitives packages would be a good idea. Then the ones that are Start focused in MK.

@thetarnav
Copy link
Member

Ideally, I should probably create a provider and hook for a context, much how it's done with Kobalte UI.

Feel free to propose further changes in new issues/PRs.

I'm going to use your changes so far to fix the existing package and rename it to cookies so that it separated from solid start.

@thetarnav thetarnav merged commit 4338fa9 into solidjs-community:main Sep 30, 2024
3 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.

3 participants