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.

Sign up for GitHub

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

Jump to bottom

docs: add example of dynamic layout with static page #922

Merged
merged 4 commits into from
Oct 3, 2024
Merged

docs: add example of dynamic layout with static page #922

merged 4 commits into from
Oct 3, 2024

Conversation

tylersayshi
Copy link
Contributor

you can see the time change as navigating the site which shows that the layout is indeed dynamic

The rest of the home page stays unchanged and can be cached

Example for #885

@tylersayshi
docs: add example of dynamic layout with static page
23c60c3 
you can see the time change as navigating the site which shows
that the layout is indeed dynamic

The rest of the home page stays unchanged and can be cached

Example for dai-shi#885
@vercel Vercel
Copy link

vercel bot commented Sep 30, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
waku ⬜️ Ignored (Inspect) Visit Preview Oct 3, 2024 3:48am

@codesandbox-ci CodeSandbox CI
Copy link

codesandbox-ci bot commented Sep 30, 2024 •
edited
Loading

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.

dai-shi
dai-shi reviewed Sep 30, 2024
View reviewed changes
examples/25_weave_render/src/entries.tsx
path: '/foo',
component: FooPage,
}),
]);
Copy link
Owner

Choose a reason for hiding this comment

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

How about adding another route like /nested/bar which has static layout and dynamic page?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added... there is weird behavior though

When I nav to /nested/bar then click the link to go back to home or foo, I get a white screen with no content. Is this a bug? Or did I make a mistake?

Copy link
Owner

Choose a reason for hiding this comment

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

Does it happen both in DEV and PRD? Can be a bug.

Copy link
Owner

Choose a reason for hiding this comment

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

Wait, /nested/bar still has a dynamic root layout?

I think we should make:

  • /foo: static layout + dynamic page
  • /nested/bar: static layout + dynamic layout + static page

what do you think?

Copy link
Contributor Author

@tylersayshi tylersayshi Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Does it happen both in DEV and PRD? Can be a bug.

Yes it happens both

Copy link
Contributor Author

Choose a reason for hiding this comment

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

static layout + dynamic layout + static page

How do I make two layouts on a route?

Copy link
Owner

Choose a reason for hiding this comment

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

/nested/_layout.tsx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I changed it. let me know if that's what you meant though. I'm not super confident with it yet.

Copy link
Owner

Choose a reason for hiding this comment

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

It looks different. Okay, we may not need nested. How about this?

  createLayout({
    render: 'static',
    path: '/',
    component: HomeLayout,
  }),

  createPage({
    render: 'static',
    path: '/',
    component: HomePage,
  }),

  createLayout({
    render: 'static',
    path: '/foo',
    component: FooLayout,
  }),

  createPage({
    render: 'dynamic',
    path: '/foo',
    component: FooPage,
  }),

  createLayout({
    render: 'dynamic',
    path: '/bar',
    component: BarLayout,
  }),

  createPage({
    render: 'static',
    path: '/bar',
    component: BarPage,
  }),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, makes sense ! lgtm - changed

@tylersayshi
Copy link
Contributor Author

@dai-shi is your general experience with the _layout pattern positive? Or is it not something you use in practice very much?

@dai-shi
Copy link
Owner

dai-shi commented Oct 2, 2024

@dai-shi is your general experience with the _layout pattern positive? Or is it not something you use in practice very much?

I'm not quite sure if my usage would be similar to everyone's. With fs-router, the root layout would be necessary, but in general, I wouldn't need to use other layouts unless I care performance much.

There are a few things I would like to reconsider, and you can probably help on them:

  • separation of unstable_defineRouter and createPages
    • currently, it's a bit confusing. defineRouter can be more primitive, or it's probably just naming.
  • improve client cache system with defineRouter
    • Currently it's only layout/layout/page pattern, which is too related with createPages.
    • Components could be freely placed, independent from route path.
  • Likewise, shouldSkip mechanism with defineRouter should be revisited.
    • It's hardly undestandable.

@tylersayshi
Copy link
Contributor Author

@dai-shi is your general experience with the _layout pattern positive? Or is it not something you use in practice very much?

I'm not quite sure if my usage would be similar to everyone's. With fs-router, the root layout would be necessary, but in general, I wouldn't need to use other layouts unless I care performance much.

There are a few things I would like to reconsider, and you can probably help on them:

  • separation of unstable_defineRouter and createPages

    • currently, it's a bit confusing. defineRouter can be more primitive, or it's probably just naming.

  • improve client cache system with defineRouter

    • Currently it's only layout/layout/page pattern, which is too related with createPages.
    • Components could be freely placed, independent from route path.

  • Likewise, shouldSkip mechanism with defineRouter should be revisited.

    • It's hardly undestandable.

all sounds good to me! I don't have immediate thoughts/ideas for how to approach changing those, but I'll be thinking about what makes sense for them.

dai-shi
dai-shi reviewed Oct 3, 2024
View reviewed changes
examples/25_weave_render/src/entries.tsx
import BarLayout from './components/BarLayout';
import FooLayout from './components/FooLayout';

// The use of `lazy` is optional and you can use import statements too.
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can remove this lazy usage from the examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see you merged it, should I make a second PR to remove this?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, please go ahead. I mean from all examples.

@dai-shi dai-shi merged commit 5e183b8 into dai-shi:main Oct 3, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dai-shi dai-shi dai-shi left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tylersayshi @dai-shi