Skip to content

Conversation

@tylersayshi
Copy link
Member

Moving to new_createPages

I left createPages in the docs since we will drop the existing createPages soon and replace it with new_createPages under the same name.

@vercel
Copy link

vercel bot commented Nov 11, 2024

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 Nov 12, 2024 0:27am

@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 11, 2024

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.

@tylersayshi
Copy link
Member Author

with the tests failing I'm seeing new_defineRouter needs to be mocked

@tylersayshi tylersayshi marked this pull request as draft November 11, 2024 05:53
@tylersayshi
Copy link
Member Author

I am currently thinking through how best to test renderRoute in place of getComponent

https://github.com/dai-shi/waku/blob/main/packages/waku/tests/create-pages.test.ts#L531

also, in place of the skip tests: we should probably eventually add tests for the new define router.

@tylersayshi

This comment was marked as resolved.

@tylersayshi tylersayshi marked this pull request as draft November 11, 2024 08:06
@tylersayshi tylersayshi changed the title refactor: use new_createPages in examples and e2e tests refactor: use new_createPages in examples Nov 11, 2024
@tylersayshi
Copy link
Member Author

react tweet example seems fine

weave render does not render anything when navigating away from, then back to home 🤔

Comment on lines 44 to 49
<Link
to={'/nested/bar' as never}
to={'/bar'}
pending={<Pending isPending />}
notPending={<Pending isPending={false} />}
>
Nested / Bar
Bar
</Link>
Copy link
Member Author

Choose a reason for hiding this comment

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

@dai-shi is this bad link here intentionally?

the as never shows me that the type error was known, but I can't think of a reason why we would want a broken link in the example.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure your original intent, but I thought it's an intentional 404 link.

Copy link
Member Author

Choose a reason for hiding this comment

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

changed the link back but changed the text to make it obvious that link will 404 if that's ok with you

@tylersayshi tylersayshi marked this pull request as ready for review November 11, 2024 20:45
Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

The change looks good to me.

@dai-shi dai-shi merged commit f843bb8 into wakujs:main Nov 12, 2024
26 checks passed
@dai-shi dai-shi mentioned this pull request Nov 13, 2024
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.

2 participants