sveltejs / kit

web development, streamlined
https://svelte.dev/docs/kit
MIT License
18.73k stars 1.94k forks source link

Improve `adapter-cloudflare`'s exclusion logic for prerendered paths #12649

Closed jasongitmail closed 3 weeks ago

jasongitmail commented 2 months ago

Describe the bug

Problem

My app exceeds the 100 combined include & exclude routes allowed by Cloudflare.

This is because I prerender various blog routes, such as /blog/tag/orange, etc.

Theoretically I can adjust the default routes rules:

routes: {
    include: ['/*'],
    exclude: ['<all>']
},

The approach would be to remove <all> and use the individual allowed values but omit <prerendered>:

routes: {
    include: ['/*'],
    exclude: ['<build>', '<files>', '/blog/tag/*']  
},

The problem is, now I'll need to manually update the include array for other individual prerendered pages, and remember to keep this updated forever, any time I add or remove paths or change the rendering strategy (e.g. /, /faq, /pricing, /terms, etc.)

Suggested solution

  1. I notice many of the autogenerated exclude rules look like this:

    It appears SvelteKit could safely exclude all "*__data.json" files by default, correct?

  2. For parameterized routes (like /blog/[slug], /blog/tag/[tag]), do not generate an item in the <prerendered> list for every single prerendered page, instead, generate a regex rule for parameterized routes (e.g. /blog/*, /blog/tag/*). This would solve the issue in 99% of cases, I'd guess.

Reproduction

n/a

Logs

No response

System Info

npmPackages:
    @sveltejs/adapter-cloudflare: ^4.7.2 => 4.7.2 
    @sveltejs/enhanced-img: ^0.2.0 => 0.2.0 
    @sveltejs/eslint-config: ^7.0.1 => 7.0.1 
    @sveltejs/kit: ^2.5.25 => 2.5.25 
    @sveltejs/vite-plugin-svelte: ^3.1.0 => 3.1.0 
    svelte: ^5.0.0-next.123 => 5.0.0-next.123 
    vite: ^5.2.11 => 5.2.11

Severity

annoyance

Additional Information

No response

nevil2 commented 2 months ago

Not sure it helps, but '<all>', '<build>' and '<files>' seems to create a long list of files names, each of which seems to count as a rule. So I have:

routes: {
    include: ['/route1/*', 'route2/*/*', ..... ],
    exclude: []  
},

where the routes are the ones which are not prerendered.

eltigerchino commented 3 weeks ago

We decided that it was not safe to use a wildcard to exclude many routes at the same time on behalf of the user. For this reason, we've made the exclusion logic configurable. You can choose to use the wildcard for this purpose if it helps with the deployment of your blog.

jasongitmail commented 3 weeks ago

@eltigerchino

Why would SvelteKit not exclude */__data.json by default?

IMO, these clearly should be excluded by default for a good DX, given these files are autogenerated by SvelteKit and they bloat the generated path inclusion list by creating 2 entries for every single pre-rendered path, when we know __data.json is not dynamic and could be excluded by default.

I don't understand the reasoning to leave this as in and let other devs bump into this headache again.

eltigerchino commented 3 weeks ago

I think this comment provides some history on this topic https://github.com/sveltejs/kit/issues/8827#issuecomment-1412572243 Similarly, this issue is already open here https://github.com/sveltejs/kit/issues/9616

jasongitmail commented 3 weeks ago

Respectfully, that doesn't answer my question. __data.json should be excluded and the links in your message do not provide any argument against doing so.

I'll copy over details from my OP of this issue into the 2nd linked issue, so the feedback on this issue isn't lost.