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

chore(packages): patch deps and use built-in solid type #4

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

onx2
Copy link

@onx2 onx2 commented Jun 11, 2024

I'd like to chip away at updating some dependencies and minor changes if that's okay with you.

  • Use npx npm-check-updates --target patch -u to patch all dependencies
  • Use ParentProps exported type from solid-js for children prop instead of custom implementation
  • Use splitProps from solid-js to prevent any reactivity loss issues
  • Use boolean flag instead of number for clearer intent and less memory
  • Resolves Can't be used around solid-js/router #3

This seems to work now for me when I tested locally.

render(
  () => (
    <I18nProvider {...i18nState}>
      <Router root={App}>{routes}</Router>
    </I18nProvider>
  ),
  document.getElementById("root")!
);

@onx2 onx2 marked this pull request as ready for review June 11, 2024 16:31
@eyelly-wu
Copy link
Contributor

@onx2 Thank you for your contribution. I was going to merge your PR, but the current performance shows that it does not pass the test cases. Please make sure the test cases pass before trying again.

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.

Can't be used around solid-js/router
2 participants