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

feat: entries gen for fs router projects #886

Merged
merged 5 commits into from
Sep 29, 2024

Conversation

tylersayshi
Copy link
Contributor

@tylersayshi tylersayshi commented Sep 16, 2024

This PR will add a vite plugin for running on waku dev that will generate entries.gen.tsx

Tasks left

  • Attempt to find prettier config
  • Test with more calls to getConfig
  • Disable generation when not using fs-router

Copy link

vercel bot commented Sep 16, 2024

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

Name Status Preview Updated (UTC)
waku ✅ Ready (Inspect) Visit Preview Sep 28, 2024 10:52pm

Copy link

codesandbox-ci bot commented Sep 16, 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.

Copy link
Owner

@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.

Welcome to the vite world.

packages/waku/src/config.ts Outdated Show resolved Hide resolved
packages/waku/src/lib/utils/vite-loader.ts Outdated Show resolved Hide resolved
packages/waku/src/lib/plugins/vite-plugin-typegen.ts Outdated Show resolved Hide resolved
packages/waku/src/lib/plugins/vite-plugin-typegen.ts Outdated Show resolved Hide resolved
packages/waku/src/lib/plugins/vite-plugin-typegen.ts Outdated Show resolved Hide resolved
packages/waku/src/lib/plugins/vite-plugin-typegen.ts Outdated Show resolved Hide resolved
@tylersayshi
Copy link
Contributor Author

Commenting my findings from a bit of research so far here

This plugin from the tanstact router is very similar to what we will want. It looks like they just use watchChange to trigger their check to generate their routes file:

https://github.com/TanStack/router/blob/main/packages/router-plugin/src/core/router-generator-plugin.ts#L79-L83

I was unable to get watchChange logging in my initial tests with out setup, but this leads me to think that it may be how we configure the vite config for the dev server maybe? Either way, it's a POC that watchChange should work for what we want to do.

The downside of their example is that afaict: They do not have a need to import anything like we do for our getConfig.

Since at this stage we are really just interested in collecting the routes for some autocomplete on the link paths, I think our best first option is to start with defaulting to string for any slug. This makes it so that we can get away with just reading filename and building the paths as a reflection of those filenames.

I'll put together a draft of what this will look like tomorrow night and in doing so get rid of any notion of importing from the plugin to run getConfig.

@tylersayshi
Copy link
Contributor Author

Another thing to add: I no longer think that making a generation of the entries file makes sense. With it just being a plugin that looks at what path you are on, we won't know the render mode of any of the routes, so making an entries.gen.tsx won't make sense.

Instead I'll make something like paths.gen.ts that has all possible paths in the app for the paths defined in src/pages assuming all slugs could be any string

Copy link
Contributor Author

@tylersayshi tylersayshi left a comment

Choose a reason for hiding this comment

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

My TODO that I am planning for:

  • Test with more types of getConfigs
  • clean up the code in the vite plugin file a bit
  • run prettier on the generated output programatically
  • fix the vite plugin to hot reload the generation when necessary

examples/05_nossr/src/pages/about.tsx Show resolved Hide resolved
packages/waku/src/lib/plugins/vite-plugin-typegen.ts Outdated Show resolved Hide resolved
packages/waku/src/lib/plugins/vite-plugin-typegen.ts Outdated Show resolved Hide resolved
examples/05_nossr/entries.gen.tsx Outdated Show resolved Hide resolved
examples/05_nossr/entries.gen.tsx Outdated Show resolved Hide resolved
Copy link
Owner

@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.

random replies.

examples/05_nossr/entries.gen.tsx Outdated Show resolved Hide resolved
examples/05_nossr/src/pages/about.tsx Show resolved Hide resolved
packages/waku/src/lib/plugins/vite-plugin-typegen.ts Outdated Show resolved Hide resolved
packages/waku/src/lib/utils/vite-loader.ts Outdated Show resolved Hide resolved
Copy link
Owner

@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.

Looking great!

import { createPages } from 'waku';
import type { PathsForPages } from 'waku/router';

import Layout, { getConfig as Layout_getConfig } from './src/pages/_layout';
Copy link
Owner

Choose a reason for hiding this comment

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

What will it be like when it's layout.tsx instead of _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.

Do we support both layout.tsx and _layout.tsx? If yes, what happens if a project has both in the same directory?

Copy link
Owner

Choose a reason for hiding this comment

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

layout.tsx works the same as foo.tsx. So it will be /layout like /foo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! that would have been in conflict. I fixed this with _Layout for the actual layouts' names and added a page named /layout to the fs-router example

examples/05_nossr/src/pages/_layout.tsx Show resolved Hide resolved
examples/05_nossr/src/pages/about.tsx Show resolved Hide resolved
packages/waku/src/lib/plugins/vite-plugin-typegen.ts Outdated Show resolved Hide resolved
packages/waku/src/lib/plugins/vite-plugin-typegen.ts Outdated Show resolved Hide resolved
packages/waku/src/lib/middleware/dev-server-impl.ts Outdated Show resolved Hide resolved
@tylersayshi
Copy link
Contributor Author

Found the issue in the playwritght tests.... the vite plugin is creating bad paths in windows 😅

dang windows

Error: ENOENT: no such file or directory, open 'D:/a/waku/waku/examples/04_cssmodules/src/pagesD:\a\waku\waku\examples\04_cssmodules\src\pages\about.tsx'

@tylersayshi
Copy link
Contributor Author

@dai-shi I have the fixes for above locally... a few last questions

  1. Do you want entries.gen.tsx added to each gitignore? (top level of monorepo and all example templates)
  2. Do you still want to add as const to the examples? I think it is good to have to make it more clear how to get the types working, but up to you.
  3. should the generated imports use the .js import style that we use internally?

@dai-shi
Copy link
Owner

dai-shi commented Sep 28, 2024

the vite plugin is creating bad paths in windows 😅

Yes, that's a tough one. We spent so many hours and even days fighting with it with @himself65 .

@dai-shi
Copy link
Owner

dai-shi commented Sep 28, 2024

Do you want entries.gen.tsx added to each gitignore? (top level of monorepo and all example templates)

Only top level. As we discussed, in the template, we expect people to commit entries.gen.tsx so that CI can type check, right?

Do you still want to add as const to the examples? I think it is good to have to make it more clear how to get the types working, but up to you.

Yes, for all TS examples.

should the generated imports use the .js import style that we use internally?

No. It can be like examples/07_router/src/entries.tsx.

@tylersayshi
Copy link
Contributor Author

No. It can be like examples/07_router/src/entries.tsx.

I think you misunderstood what I was referencing here. I mean import .. from './_layout.js' for example

@dai-shi
Copy link
Owner

dai-shi commented Sep 28, 2024

No. It can be like examples/07_router/src/entries.tsx.

I think you misunderstood what I was referencing here. I mean import .. from './_layout.js' for example

Isn't it about the code in entries.gen.tsx?

@tylersayshi
Copy link
Contributor Author

can be like examples/07_router/src/entries.tsx.

I think you misunderstood what I was referencing here. I mean import .. from './_layout.js' for example

Isn't it about the code in entries.gen.tsx?

this at the top of that file:

import _Layout, { getConfig as _Layout_getConfig } from './pages/_layout.js';
import About, { getConfig as About_getConfig } from './pages/about.js';
import Index, { getConfig as Index_getConfig } from './pages/index.js';

@dai-shi
Copy link
Owner

dai-shi commented Sep 28, 2024

Yes, we can omit .js and should do to be consistent with examples/07_router/src/entries.tsx. (Ah, this one is using dynamic import. We can change it to static import.)

@tylersayshi
Copy link
Contributor Author

Yes, we can omit .js and should do to be consistent with examples/07_router/src/entries.tsx. (Ah, this one is using dynamic import. We can change it to static import.)

Sounds good, but there's no dynamic import? We dropped that idea a while ago

@tylersayshi
Copy link
Contributor Author

as I went through each of the examples, I noticed that most use the entries.tsx file. Is that something you think should be recommended on waku.gg as the standard?

My current understanding of the docs is that it only really suggests the fs-router

@dai-shi
Copy link
Owner

dai-shi commented Sep 28, 2024

but there's no dynamic import?

never mind. It was referring to examples/07_router/src/entries.tsx.

@dai-shi
Copy link
Owner

dai-shi commented Sep 28, 2024

as I went through each of the examples, I noticed that most use the entries.tsx file. Is that something you think should be recommended on waku.gg as the standard?

My current understanding of the docs is that it only really suggests the fs-router

So, there are two aspects (or goals) in Waku.

  1. Research project: What would be a "minimal" API for an RSC framework.
  2. Production-level framework: Let's make React development fun again.

Obviously, I started as the first one, and I'm not thinking it a toy. My vision includes Jotai integration, multiple routers (router-agnostic API), and building ecosystem.
But, many people get attracted to the second one, and it's our current focus. Particularly, waku/router plays an important role.

examples/0[1234]* are for the second aspect.
examples/0[57]* are still for the second aspect, but for advanced users.
examples/0[89]* are still for the second aspect, but more like PoC. (Maybe we should convert them to use waku/router.)
examples/[12]* are for the first aspect, or some experiments for the second.

Copy link
Owner

@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.

Looks great! Outstanding work!

@dai-shi dai-shi merged commit 46a44cf into dai-shi:main Sep 29, 2024
28 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.

2 participants