unjs / nitro

Next Generation Server Toolkit. Create web servers with everything you need and deploy them wherever you prefer.
https://nitro.unjs.io
MIT License
5.5k stars 472 forks source link

Disallow `prerender: true` route rule for route rules having `*` #1856

Open pi0 opened 8 months ago

pi0 commented 8 months ago

(context: from chat with @JamieCurnow)

We only support prerender: true for static route rules and omit the rest. (source: https://github.com/unjs/nitro/blob/7a6c61dbd1453e7bef69f999d0325881f901e8f6/src/prerender.ts#L32)

Using typescript, could be nice to make it clear.

 routeRules: {
    '/**': { prerender: true }, // invalid
    '/blog/**': { prerender: true }, // invalid
    '/blog/test': { prerender: true }, // valid
  },
Hebilicious commented 8 months ago

That's fair. Could we introduce a feature to support something like this ?

routeRules: {
    '/blog/**': { prerender: [1,2,3] }, // array syntax 
    '/articles/**': { prerender: getPages() } // dynamic, user provided function called before the prerender runs that must returns an array of string.
  }
pi0 commented 8 months ago

For route rules, they should remain JSON serializable in order to keep them universal.


Out of context of this issue:

We could also have async function support for prerender.routes as well as async routeRules(). We initially also prevented async function APIs in Nuxt 3 and Nitro because it can also lead to bad practices such as slow configuration loading due to extra async stuff and also bloating configuration with business logic (such as connecting to an external CMS to fetch the pages)

We were talking about possibility of supporting prender.ts that is a (prerender) runtime entry that allows async fetching of pre-rendered routes with access to all runtime data (cms config, runtime config, fetch instance, etc)

Hebilicious commented 8 months ago

For route rules, they should remain JSON serializable in order to keep them universal.

Out of context of this issue:

We could also have async function support for prerender.routes as well as async routeRules(). We initially also prevented async function APIs in Nuxt 3 and Nitro because it can also lead to bad practices such as slow configuration loading due to extra async stuff and also bloating configuration with business logic (such as connecting to an external CMS to fetch the pages)

We were talking about possibility of supporting prender.ts that is a (prerender) runtime entry that allows async fetching of pre-rendered routes with access to all runtime data (cms config, runtime config, fetch instance, etc)

I think keeping them serialisable is a good choice, but what do you think of the array syntax in this context? We could allow the wildcard if prerender is a string array, for simple use cases.

I like prerender.ts, but also maybe a dedicated hook could work too ? Like nitro:prerender. For Nuxt we currently have to use nitro:config which isn't the most elegant.

In both cases we should consider having a dedicated prerender section in the docs to explain this.

JamieCurnow commented 8 months ago

@pi0 is this possible with Typescript? I've made this example to try this out (which doesn't work):

type IncludesAstrix<S> = S extends `${infer F}*${infer T}` ? true : false;

// test IncludesAstrix (hover over):
type Truthy = IncludesAstrix<'with/a/*'>
type Falsey = IncludesAstrix<'some/route'>

type RoutesObject = {
  [K in string]: IncludesAstrix<K> extends true ? number : string;
};

const routes: RoutesObject = {
 'some/route': 'a string',
 'with/a/*': 'should not be string!' // this should error?
}

Typescript Playground

I've asked on the TS discord for some advice, and this is what I got:

since your indexing operation is [K in string], K will always just be string, not something more specific, and since IncludesAstrix is false it always goes down the else branch in that conditional type if you hover over RoutesObject you can see what it reduces to the "any other key should be a string" part of your requirements is going to be a fundamental issue. there's just no way to express the type "any string that doesn't contain a *" in typescript's type system


It could be done if routeRules was a function though:

type IncludesAstrix<S> = S extends `${string}*${string}` ? true : false

type RoutesObject<K extends string> = {
  [K1 in K]: IncludesAstrix<K1> extends true ? number : string
}

const routeRules = <K extends string>(routes: RoutesObject<K>) => routes

routeRules({
 'some/route': 'a string',
 'another/route/*': 42,
 'bad': 1, // does error
 'bad/*': 'should not be string!', // does error
})

Typescript Playground

But that would obviously mean a change to the nitro config signature... What do you think @pi0 ?

JamieCurnow commented 8 months ago

Out of pure curiosity. Is there a reason that we skip wildcards for pre-rendering? Wildcards do work when targeting routes to not pre-render when they should through crawling eg:

https://stackblitz.com/edit/github-q9zy7q-tc3agj?file=nitro.config.ts

See the message thread: https://discord.com/channels/1065212533444726784/1129811416266854543/1167213407901077576