-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
Client-side routing breaks layouts #113
Comments
@Blankeos Maybe there is an issue with the cumulative +Layout implementation? |
Looking into this! |
Hi @jaydevelopsstuff. Looked into your MRE, thanks for this, I could play around with the issue immediately! Found that this isn't a Vike issue. It's just a very common misuse of SolidJS if you're coming from React. TL;DR; You're destructuring props. Don't do that. I have some tips below though. I made a PR on your repo btw for the fix. Few tips about SolidJS and props
// ❌ bad
import { FlowProps } from "solid-js"
const MyComponent = (props: FlowProps) => {
const { children } = props;
// ...
}
// ❌ bad
import { FlowProps } from "solid-js"
const MyComponent = ({ children }: FlowProps) => {
const { children } = props;
// ...
}
// ✅ good (also personally useful because you know which ones are props from a glance)
import { FlowProps } from "solid-js"
const MyComponent = (props: FlowProps) => {
// ...
}
// ✅ good (if you really want to destructure)
import { mergeProps, splitProps, FlowProps } from "solid-js"
const MyComponent = (props: FlowProps<{ age?: number }>) => {
// 1. First way: always use if you want to destructure and also add default values.
// I cannot confirm if this works actually lol, so I recommend the 2nd way.
const { children, age } = mergeProps({ age = 14}, props)
// 2. Second Way: or still keep it inside a parent object (I like this personally because of the same reason above)
const _props = mergeProps({ age = 14}, props)
// 3. Third way: Similar const { children, age, ...restProps } = props;
const [otherProps, restProps] = splitProps(props, ["age"]); // otherProps has `age`. restProps has `children`, and etc.
// ...
} Also some improvements, I could add in your code (this doesn't matter though, I just realized):
By the way if this helped you, I'll just plug my SolidJS + Vike boilerplates here:
|
@Blankeos Great answer 💯 and makes sense about deconstructing props. Although I ain't sure I understand the issue about
These look great! We could add them to Also, if you're up for it, you could co-lead |
Thanks so much for the help, just a dumb mistake. |
Happy to help @jaydevelopsstuff. It's perfectly okay to make that mistake. It's just the React-to-Solid jet lag haha. I made the same mistake before lol. Also, hi @brillout, I think I missed reading your reply haha.
Yup, based on the file structure docs, I never really thought Will probably still stick with
Really appreciate that coming from you! Sounds awesome! Yeah, I think it would help the Vike + Solid community a lot if it's on the examples.
It would be a really cool opportunity, but not sure if I'm ready for the obligation. What does the co-lead usually do? Feel free to dm me on Discord! In any case, I'll do my best answering questions in the community. I think it helps everyone and the framework in the long term. Would really love to see Vike becoming mainstream with giants like NextJS and upcoming ones like Tanstack Router. |
You can contribute however you want and feel like. Examples:
|
When using client side navigation (e.g. clicking anchor links), Vike fails to add/remove Layouts from the page.
Screen.Recording.2024-09-02.at.7.19.40.PM.mov
MRE Repo
pnpm dev
and open in browserThe text was updated successfully, but these errors were encountered: