frandiox / reactesse-edge-template

MIT License
28 stars 1 forks source link

Use same structure for functions/{props,api} as src/pages? #2

Closed jfsiii closed 3 years ago

jfsiii commented 3 years ago

I'd rather not have nested routes for src/pages but flat for functions/props. Can the names/structure set in src/pages/**/* also work in functions/props/**/* and functions/api/**/*?

For example, with pages like

 tree src/pages/
src/pages/
├── README.md
├── [...all].tsx
├── about.mdx
├── hi
│   └── [name].tsx
└── index.tsx

is this structure supported

 tree functions/props/
functions/props/
├── [...all].tsx
├── hi
│   └── [name].ts
└── index.ts

I looked at https://vitedge.js.org/props.html#props-getter-locations and tried to set meta.propsGetter but only some of them worked. I can follow with steps to reproduce, but I wondered if it was even supported.

jfsiii commented 3 years ago

For some more detail, if I use onRoutesGenerated in vite.config.js to update the routes, then log contents of import routes from 'virtual:generated-pages' in src/main.ts I get:

[
  {
    path: '/hi/:name',
    component: [Function: Hi],
    exact: true,
    meta: {
        propsGetter: 'hi/[name]'
    }
  },
  {
    path: '/',
    component: [Function: Home],
    exact: true,
    meta: { 
        propsGetter: 'index' 
    }
  },
  {
    path: '/about',
    component: [Function: MDXContent] { isMDXComponent: true },
    exact: true,
    meta: {
        propsGetter: 'about' 
    }
  },
  {
    path: '*',
    component: [Function: NotFound],
    exact: true,
    meta: {
      propsGetter: '[...all]'
    }
  }
]

then do npm run dev, http://localhost:3333/ will not call props/index,

Screen Shot 2021-08-25 at 10 01 12 PM

but http://localhost:3333/hi/JFSIII does call props/hi/[name]

Screen Shot 2021-08-25 at 10 01 26 PM

However, with npm run preview, neither work and props/hi/JFSIII 404's

Screen Shot 2021-08-25 at 10 04 51 PM

I understand it might not be supported, so I don't want to overwhelm with info. Let me know if you need more (e.g. push an example repo or something)

frandiox commented 3 years ago

The official support I wanted to make is having a flat structure in function/props, which is based on route name (and can be overriden by meta.propsGetter).

Therefore, the original idea is that, even if you have nested routes, they would have a unique, flat name (perhaps /users/[id] has a name/propsGetter user-details). This should work and if it doesn't then we need to fix it.

I've seen some other people also using names/propsGetter with slashes, such as the one you mentioned hi/[name] (with a handler in functions/hi/[name].js.This was not my initial intention and haven't tested it well, but I think it should work. Normally, I just turn the slashes into dashes, like in this line here.

then do npm run dev, http://localhost:3333/ will not call props/index,

I'm curious about this one, since I think it should work 🤔 I've tested it in the template with route.name = 'index' or route.meta.propsGetter = 'index', and looks like it works. Can you provide a repro for this?


The function/api folder is completely unrelated to frontend routes or props. This folder uses a different routing strategy and supports slashes /api/a/b/c without issues.

jfsiii commented 3 years ago

I pushed a repo https://github.com/jfsiii/reactesse-use-deep-props to show the changes & I made and behavior I'm seeing

jfsiii commented 3 years ago

The function/api folder is completely unrelated to frontend routes or props. This folder uses a different routing strategy and supports slashes /api/a/b/c without issues.

I thought so. It seems like it, but I wanted to make sure. It's that mismatch (pages & api use A, props uses B) that was confusing and I'm trying to avoid.

I see the value in a flat or different approach (and this is your opinionated starter), I just don't want to use that approach now and wondered if it was supported. Thanks for taking a look.

jfsiii commented 3 years ago

@frandiox I made another https://github.com/jfsiii/vitedge-from-react-ts-template starting with the react-ts template from https://vitedge.js.org/getting-started.html#installation and adding things step by step.

In that one, the I set meta.propsGetter to both flat and nested/dynamic values but they assume/require a .js extension. props/**/*.ts files fail

routes.ts https://github.com/jfsiii/vitedge-from-react-ts-template/blob/ef48925cf52316e4c60402706b1b88754d851b89/src/routes.ts#L11-L18 ``` { path: '/users/:id', exact: true, component: () => import('./pages/users/[id]'), meta: { // propsGetter: 'js-works', // propsGetter: 'ts-fails', propsGetter: 'users/[id]', // works for .js; not .ts }, } ```
Screenshots showing failures Screen Shot 2021-08-26 at 6 14 58 PM Screen Shot 2021-08-26 at 6 15 43 PM Screen Shot 2021-08-26 at 6 16 21 PM Screen Shot 2021-08-26 at 6 19 47 PM
frandiox commented 3 years ago

@jfsiii So I've fixed the following issues:

With that, it should be possible to call props "index" or "[whatever]". Apart from that, in your second repo I think the only problem is that you are calling vite CLI instead of vitedge, which adds some Node flags and TS stuff automatically. This is shown in the docs/usage but I guess we should mention it in the getting starter as well 🤔

Closing it for now. The fixes will be released in the next version soon. Thanks for all the help!

frandiox commented 3 years ago

Fixes are released in 0.16.1 🎉

jfsiii commented 3 years ago

Thanks! I pulled 0.16.1 and copy-pasted package.json changes. Everything worked great!