-
-
Notifications
You must be signed in to change notification settings - Fork 348
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
V1 Design #578
Comments
Treat the following as notes, not explicit feedback or requests. singleConfigFile contradicts the nested example?
nested layout API via an arrayusing an array instead of a string for the const adminPagesConfig = {
title: 'Admin Panel',
// the first layout is the outer layout
Layout: [
'./layouts/RootLayout.vue',
'./layouts/LayoutAdminPages.vue',
// renders
// <RootLayout>
// <LayoutAdmingPages>
// {children}
// </LayoutAdminPages>
// </RootLayout>
]
} nested pages that define additional layouts can be merged easily with // parent
export default {
// additional parameters
Layout: ["./Layouts/RootLayout.tsx"]
}
// renders ["./Layouts/RootLayout.tsx"] // page1
export default {
// additional parameters
Layout: [
"./Layouts/Product.tsx",
"./Layouts/Info.tsx"
]
}
// renders parent.Layout.concat(page1.Layout) =
// ["./Layouts/RootLayout.tsx", "./Layouts/Product.tsx", "./Layouts/Info.tsx"] // page2 is a child of page1
export default {
// additional parameters
Layout: ["./Layouts/Sidenav.tsx"]
}
// renders parent.Layout.concat(page1.Layout).concat(page2.Layout) =
// ["./Layouts/RootLayout.tsx", "./Layouts/Product.tsx", "./Layouts/Info.tsx", "./Layouts/Sidenav.tsx"] the nested layout example seems unintuitiveDefining the layout for nested pages in a page config seems unintuitive. Either each page is responsible for defining its layouts, or one config that defines everything. (I assume that export default {
// ...
nested: [
// why are layouts defined for sub-routes in a page config?
{
route: '/product/@id/details',
View: './DetailsView.js'
},
]
} |
I've updated the RFC to better reflect this. |
I'm not doing any SSR right now so I can't give feedback on that side. Just taking the opportunity to suggest another syntax for config: import type { Config } from 'vite-plugin-ssr'
export const config: Config = {
// ...
} This could avoid a lot of interop issues I think. |
@ArnaudBarre 👍 From a TypeScript/user perspective I believe that would work equally well. I think it mostly boils down to a preference of having |
regarding the so it would completely replace & deprecate the current anyway, it looks great, especially the custom exports ! congratz |
HTML-only: // +config.ts
import type { Config } from 'vite-plugin-ssr'
export default {
configDefinitions: [
{
name: 'htmlOnly',
handler(value) {
if (typeof value !== 'boolean') throw new Error('Config htmlOnly should be a boolean')
if (value) {
return {
name: 'Page',
// Akin to .page.server.js
env: 'SERVER_ONLY'
}
} else {
return {
name: 'Page',
// Akin to .page.js
env: 'CLIENT_AND_SERVER'
}
}
}
]
} satisfies Config // /pages/about/+config.js
export default {
Page: './Page.jsx',
htmlOnly: true
} // /pages/about/Page.jsx
// This file is loaded only on the server
export default function() {
return <>I'm a HTML-only page.</>
} FYI, the idea long term is to have frameworks define such configs. For example a VPS user is currently implementing a framework with island architecture. With V1 it works like this: // node_modules/some-framework/+config.ts
import type { Config } from 'vite-plugin-ssr'
export default {
configDefinitions: [{
name: 'island',
handler(value) {
if (typeof value !== 'boolean') throw new Error('Config island should be a boolean')
if (value) {
return {
clientEntries: ['./client-island-hydration.ts']
}
} else {
return {
clientEntries: ['./client-classic-hydration.ts']
}
}
}
}]
} satisfies Config // /pages/about/+config.js
export default {
Page: './Page.jsx',
island: true
} // /pages/about/Page.jsx
export default function() {
return <>This page is island-hydrated</>
} // /pages/+config.js
// Default value for all pages
export default {
island: false
}
One thing I don't like about V1 is that it's not clear anymore where files are being loaded. The user has to know it. Someone reading VPS user-land code for the first time will have no clue. A workaroud is to implement IDE plugins that visually display where a file is being loaded. (Btw. contributions welcome to implement prototypes.) Also, I see it to be the framework's responsability to clearly communicate where files are being loaded. Frameworks that are well designed and simple, together with such IDE plugins, solves the problem, I believe.
The foundational benefit here is that VPS knows everything about the user's app at build-time while enabling frameworks/users comprehensive control. The For the longest time it was believed that Next.js is the right level of abstraction. This RFC puts a big question mark on that assumption. Imagine a framework that deeply integrates with GraphQL. // /pages/todo/+config.ts
import type { Config } from 'awesome-framework'
export default {
query: '{ todo { text } }',
Page: './TodoListPage.tsx',
ssr: false // Not needed for a to-do list page
} satisfies Config // /pages/todo/TodoListPage.tsx
// Ideally `awesome-framework` includes type generation
import type { Page } from './types'
export default function({ result }) {
return <>{result[0].text}</> // displays "Buy milk"
} satisfies Page Data fetching is completely taken care of by |
makes sense ! thanks for the detailed explanations |
I love this. 🤩 That doesn't mean I don't also have some questions/thoughts to add, however. In your "islands" example above, how did
|
By using VPS's Extensions API. There aren't any docs for it yet, but you can have a look at an example here.
Yes,
That's not (easily) possible because configs (e.g.
While I share the sentiment to give further control to the user, I'd rather go for "convention over configuration" for this one. At the end of the day, I don't see the added value of allowing users to configure this. But I'm open to change my mind if I can see concrete benefits.
True. I'm considering the possibility to create new pages in a succint manner: /pages/index/+Page.js
/pages/about/+Page.js
/pages/jobs/+Page.js Here, a new But I'm currently leaning towards not supporting such succint syntax. But I may reconsider if users want this. |
I'm thinking of this: // pages/+config.js
export default {
// Tell VPS to also glob `+Page.js`
glob: ['Page']
} // pages/about/+Page.jsx
export default () => <>This page was created without creating/touching any +config.js file</> I think it's a good trade-off. |
Hello everybody! I would like to give my feedback about this, and ask two questions. I'm actually using VPS to build a custom framework based on islands architecture (as you have already mentioned), so every page is server-side rendered and partial hydrated client-side. For me it's great to have a clear distinction between Anyway, also describing this within a config file can be ok, and even more powerful for other scenarios. First QuestionFirst doubt is the same mentioned here. My cases are actually like: test.page.server.tsx:
plus test.page.client.tsx:
And finally a custom route test.page.route.ts:
Reading your example I'm not sure if I got how this is gonna work with V1 purposed solution? Maybe like:
And even if I use a global +config, I assume what should I put inside it's the same as above, but for each page. Second QuestionWill be possible to read pages handlers as external packages from node_modules? Like:
Or even more, read the full page |
// pages/test/+config.ts
import type { Config } from 'vite-plugin-ssr'
export default {
route: '/test',
onBeforeRender: './onBeforeRenderTest.ts', // "init code"
onRenderHtml: './onRenderHtmlTest.ts', // "full HTML page construction"
onRenderClient: './onRenderClientTest.ts', // "hydration code"
ssr: true,
Page: './Page.ts', // You *do* need this: you can only omit it for the root pages/+config.ts
passToClient: ['pageProps']
} satisfies Config After talking with the Munich team, it seems to me that the RFC is actually quite a good match with BurdaForward's use case. Feel free to PM on discord so we can elaborate. Also, FYI, a VPS sponsor is currently building a framework VPS + Solid + Partial Hydration (ideally as a re-usable rendered like https://github.com/brillout/stem-react / https://github.com/brillout/vite-plugin-ssr/tree/main/examples/stem-react).
Yes, that's actually a primary goal of VPS V1. |
I forgot to reply about the following.
I agree, my previous comment about this:
Correct.
I'm not entirely sure what you mean here, but you'll be able to publish npm packages that include |
Great! I got it now. My only doubt is still about this: Why this is needed? Cause even right now we are not using the Page feature, but only the Render() hook My prev example about this:
So I expect to only set Otherwise if I have to set both |
@npacucci Got it. So, yes, you can omit |
Example of using the V1 design: https://github.com/brillout/vite-plugin-ssr/tree/dev/examples/v1. Note that V1 is still work-in-progess and functionalities beyond this basic example isn't supported yet. |
Nice, I like the fact that it's less magic (some might call it convention), by configuring it this way it's more predictable and readable. I do think it's weird to see the + in the config file, never seen that before. Might scare some people, but might also just be something to get used to. Small note, is there a reason why you are defining the imports with strings instead of direct imports?
|
Yea, I found it ugly at first but it grew on me and I really like it now. (FYI it's SvelteKit who introduced that idea.)
Because the closure behavior would be confusing. Many criticize this about Next.js's // /pages/+config.js
let app;
export default {
onRenderClient(pageContext) {
app; // This `app` variable is another than below
},
onRenderHtml(pageContext) {
app; // This `app` variable is another then above
}
} |
I don't see the Could you provide the complete (for this moment at least) proposed typescript definition of the config? It may also be easier to discuss in a draft PR with commit of v1 typescript definition of config. (comments are better + could be resolved) |
I agree, in that example it might lead to confusion. Still think it would be less magic/more predictable by moving the responsibility of actually importing, to the config file. |
@vchirikov I will. @leonmondria I share the sentiment here but I'm afraid directly importing in |
@brillout Here is a quick question: Is VPS going to continue supporting the current design? Note: I find the |
The
I found it ugly as well at first. But I grew to like it and I'm fairly confident you will as well. So no customization planned. As long as there are no functional regression then I think it's worth it to add a little bit of ugliness (in exchange for profound improvements, and also I've actually come to find it beautiful.) |
Just here to express that we'd also like some glob matching of routes a la #578 (comment) proposes the following syntax: // pages/+config.js
export default {
// Tell VPS to also glob `+Page.js`
glob: ['Page']
} I suggest making the glob explicit. There seems to be no benefit of implicitly matching js/ts files (what about .svelte, .vue, etc.). Furthermore, why would a + prefix be of benefit? Let users decide what to match explicitly: export default {
glob: ['*.page.{jsx|tsx}']
} |
@samuelstroschein I'll think about it. (FYI there is a tension between the pro of having flexibility and the con of having too many options a la webpack. If they don't limit flexibiilty, then strong conventions, including naming conventions, are beneficial in the long term.) |
I'm actually increasingly thinking of completely removing Filesystem Routing, including the aforementioned I understand that some folks want to build frameworks on top of VPS and want control over this. So I'm open to re-consider, but I'd like to engage into discussions: if you want Filesystem Routing then add a comment over there #630. |
I've an idea how to make FS routing work very well with the V1 design. I'll show an example. We can consider the dicussion #630 as "closed" in the meantime. |
@phiberber Seems like you're slowly appreciating the whole + thing after all :-). Beyond redbar0n's point the issue with
I like the fact that |
I still hate it and it has been one of the three reasons I avoided VPS. But I'm seeing there's not that many options of symbols other than +, suffixes are bad, prefixes are good but they should not feel cluttered, which is my main problem with the + sign. Seeing a large project tree with + signs is the same to me as looking to every ad at the Times Square, too much information to process. Colocation feels badly designed to me still, it could maybe get better with proper IDE support. +Page.ts
To me - means more like an item of a list than subtraction or something negative, it's really clean and easier to read. |
That's actually something I'd be up for making a poll about and going with the result. But still premature. Out of all options, I clearly prefer |
What about |
The + sign quickly ends up being as explicit as a I'd suggest we pause and resume the discussion after a couple of months after the old design is deprecated. That said, I'm more than looking forward to hear about concrete issues, such as paper cuts around IDE. (I actually have an idea about it but I purposely don't write about it because, so far, I don't think it's enough of a priority for us to spend time on. Important to prioritize things correctly, otherwise Vike will never hit prime time.) To sum up:
Looking forward to ship the V1 (design). |
I do also like - more than + purely aesthetically. But I think + may be better here.
I agree, I never thought about it as subtraction and addition, in terms of file names. - is well established in file names, and should be intuitive to most. But, related, an issue I have with YAML is that it’s confusing to quickly glance at something and tell whether it is a list or not. Because of - being present or not (esp. when nested). That’s could actually be an issue here too, since it would look like some folder items in the IDE indicate they are part of a list, so the (inexperienced) user may be inclined to make all files in the folder be prefixed with - to be uniform. Or vice versa. Both may conflict with this library’s operation. In all, I’m starting to come around to the + sign. Let’s just try it and see. |
Everything looks exciting ! I have one minor suggestion : export default {
configDefinitions: [
{ name: 'island', handler(value) {}},
{ name: 'foo', handler(value) {}},
]
} satisfies Config I would love to see a change to : export default defineVikeConfig({
extendConfig: {
island: { handler: (value) => {} }, //object syntax
foo : (value) => { } // function syntax
}
}) The reasons why :
|
@Hebilicious It's actually already renamed, see https://vike.dev/meta. I find "meta" better than "extendConfig", given the fact that As for the config helper, it didn't occur to me that it would help JS users. Neat & good to know. PR welcome if that's something you'd be up to. I also like the idea of an object syntax. Maybe something like that: // /pages/some-page/+config.h.js
import dataHook from './data.js'
export default defineConfig({
// object syntax
dataMeta: {
value: dataHook,
// By default `env` is 'server-only'
env: 'server-and-client'
},
// direct syntax
data: dataHook
}) // /pages/some-page/data.js
export default (pageContext) => {
/* ... */
} It's a succinct way to define both the config value and its options 👍. |
I'll open a PR then ! About meta, it's more likely that people associate it with metadata, metaverse or Facebook meta ... In what context is meta about modifying configuration ? In my opinion, the name should encapsulate something like define, extend, modify or add. |
I agree and I ain't 100% happy with the name "meta" but it's the best I could find so far. The use cases listed in https://vike.dev/meta are a good representation for what (Looking forward to your |
how about |
@brillout it will be nice to have docs about v1 on vike.dev, because for now they're unlisted :\ |
@redbar0n I think, given the context, // /pages/+config.js
export default {
// Since we are in +config.js it's fairly clear that meta relates to "meta configuration".
meta: {
// ...
}
} @vchirikov I agree and it will happen as soon as the V1 design is out of beta. (And since we increasingly recommend users to use Bati and There is one little breaking change coming for the V1 design and then it should be ready for release. So it's coming soon. |
@brillout yeah I thought about that, but then again, since a config can refer to anything then meta can refer to anything not necessarily a type of config. So with just |
Technically, |
For alternative names :
Definitely agree with @redbar0n , meta by itself doesn't indicates whats happening whatsoever (definition was better in that regard). Semantically, extend can be used for both additions and modifications. |
My vote goes to
|
I actually did consider the name As for the other suggestion, they are misnomers as they convey either extending or modifying but not both. Same for I think |
It’s intuitively understood by everyone that a custom config overrides a default config. It’s pretty much a standard. Sometimes it’s the things we think about at first that are the best solutions, because that will also be what others intuitively think first. :-) |
Tailwind had that problem with theme replacing the default config instead of modifying it, they sure had a lot of questions because people weren't used to the Not sure how much the extend prefix could be useful here, although it doesn't seem to me that it's really a config, the current state of the meta parameter looks to me more like route props than a config itself. If I'm not wrong with the above thought, I'd say meta is better than extendConfig or customConfig, although in that sense I think |
Just saw this. It gives some precedence and inspiration: https://remix.run/docs/en/main/file-conventions/remix-config#routes
|
The V1 design is out of beta and now the official design. |
The
[email protected]
design in a nutshell:I.e.
+config.js
replaces.page.js
,.page.server.js
,.page.client.js
, and.page.route.js
.It also replaces
_default.page.js
as well as VPS configurations set invite.config.js
:VPS's architecture stays the same and therefore migration is relatively simple.
Succint syntax
Under consideration is the possibility to create new pages in a succint manner:
Here, a new
+config.js
file isn't needed each time a new page is created.I'm currently leaning towards not supporting such succint syntax (in favor of "Single Config File", see next section). But I may reconsider if users want this.
Single Config File (aka Single Route File)
The
singleConfigFile
enforces the entire app to be defined in that single+config.js
file. This means that this single+config.js
file represents the entire interface between VPS and your app.Nested Views
(Aka "Nested Layouts", but "Nested Views" is a better name for this design.)
This example doesn't use
singleConfigFile
(otherwisenested
would be defined in/pages/+config.js
instead).Custom Exports
The V1 design requires Custom Exports to be registered.
More
The V1 design unlocks many more capabilities, in particular around building frameworks on top of VPS. See end of this comment.
This is a very exciting design. (In many subtle ways, you'll see when you'll use a VPS framework.)
Feedback
(Negative) feedback welcome.
Acknowledgement
🙏 @AaronBeaudoin for the fruitful conversation.
The text was updated successfully, but these errors were encountered: