more efficient _routes.json for cloudflare integration

[schummar]

Changes

Fixes #6516

The cloudflare integration tends to create large _routes.json files, which quickly hits the limit of 100 combined include/exclude entries that Cloudflare imposes even for small projects.

This PR should create smaller _routes.json by

• Creating include entries for non prerendered endpoints/pages
• Creating exclude entries only for those static files, that are matched by one of the includes

That means if you have many assets and/or static pages, instead of

{
  "include": ["/*"],
  "exclude": [
    "<too many entries>"
  ]
}

you'll get

{
  "include": [
    "/someRoute",
    "/anotherRoute/*"
  ],
  "exclude" [
    "/anotherRoute/staticPart/",
    "/anotherRoute/staticPart/index.html",
  ]
}

For cases, where the "exclude all static files" strategy produces less entries - e.g. when using server mode and not prerender any/most of the pages - the old output will be used.

There is one interesting edge case, where I am really not sure what the best solution would be. If there are no dynamic routes, the include array would be empty. Cloudflare does not allow this however. Usually you would not even need to use the integration for such a case, but if you do, we need a fallback value in include. I have chosen a magic value /MATCH_NONE_INCLUDE_PATTERN for now, but maybe someone has a better idea...

Testing

Added a test case.
Coincidentally had to fix an actual pre existing bug to make it pass 😉 (Windows file paths ended up in _routes.json)

Docs

/cc @withastro/maintainers-docs for feedback!

[changeset-bot]

🦋 Changeset detected

Latest commit: 3e766f1

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[ematipico]

Thank you! I am about to review. In the meantime, a question for you: What's /MATCH_NONE_INCLUDE_PATTERN?

[schummar]

Thank you! I am about to review. In the meantime, a question for you: What's /MATCH_NONE_INCLUDE_PATTERN?

I have it in the description:

There is one interesting edge case, where I am really not sure what the best solution would be. If there are no dynamic routes, the include array would be empty. Cloudflare does not allow this however. Usually you would not even need to use the integration for such a case, but if you do, we need a fallback value in include. I have chosen a magic value /MATCH_NONE_INCLUDE_PATTERN for now, but maybe someone has a better idea...

[ematipico]

@withastro/maintainers-docs friendly ping to review the small change in documentation

[schummar]

Thanks for the review @ematipico. I found #7188, which is also related. There unnecessary invocations for 404 pages are discussed, which should also be solved by using actual includes.

So with that in mind, should we always use includes by default and only fall back to the reverse approach when the list gets over 100 entries, while the alternative has less? Right now we would choose the approach that produces the smaller list.
Or maybe a setting?

[ematipico]

Thank you! I am about to review. In the meantime, a question for you: What's /MATCH_NONE_INCLUDE_PATTERN?

I have it in the description:

There is one interesting edge case, where I am really not sure what the best solution would be. If there are no dynamic routes, the include array would be empty. Cloudflare does not allow this however. Usually you would not even need to use the integration for such a case, but if you do, we need a fallback value in include. I have chosen a magic value /MATCH_NONE_INCLUDE_PATTERN for now, but maybe someone has a better idea...

I read the description, but I still don't understand this value. Is it just a placeholder or something that cloudflare recognises?

[schummar]

I read the description, but I still don't understand this value. Is it just a placeholder or something that cloudflare recognises?

Then I have not been clear enough, sorry. Assume you are using hybrid mode and don't opt out of prerender for any page/endpoint (yet). That means we would get this _routes.json:

{
  "version": 1,
  "include": [],
  "exclude": []
}

Cloudflare does not allow this and throws an error on deployment:

Invalid _routes.json file found at: <path>/dist
Routes must have at least 1 include rule, but no include rules were detected.

The limitation is documented: https://developers.cloudflare.com/pages/platform/functions/routing/#limits
So to solve this, I had to add some entry to include. I wanted something that will likely not match anything real, hence the weird magic value.

However, while writing this response, I had a better idea: Lets do this:

{
  "version": 1,
  "include": ["/"],
  "exclude": ["/"]
}

Still a workaround for Cloudflare's limitation but not as baffling. And it can actually not match anything since exclude trumps include.

[ematipico]

Thank you @schummar , I had a different interpretation of "magic value", but I understand now. Thank you again!

[sarah11918]

Sorry, this sentence is not clear to me, and I think it's because it is trying to say too much in one sentence with too many clauses.

This sentence should be able to clearly state, ideally each in its own sentence:

• what the _routes.json file includes (Is it an include array, an exclude array and a list of files from your dist/ folder? Or are these files now inside the include array?)
• what the relationship of the include and exclude array are.. to each other? to these other files? I'm not clear from the sentence above what is supposed to match with what.

Can you try redrafting the above into at least two sentences, because for sure we have at least two sentences worth of content there! Don't be afraid to sound overly-simplistic. I can then tighten it up once I can see the clear relationship of everything, if needed!

[schummar]

If it's ok for you, I would actually try to get less specific here. The behavior is a bit more complex than before and describing it accurately would need more space. Stating that the file will be generated "based on your applications's dynamic and static routes" might be enough for the user. And I added a link to Cloudflare's docs for more in depth information about _routes.json in general.

[schummar]

@sarah11918 did you have a chance to look at this again?

[sarah11918]

Hey @schummar ! This rewrite looks clearer, and I think it's helpful! Thanks!

[vvashisht]

This approach disregards any routes present in the functions directory and not in astro's scope. I'm using the 'directory' mode and I have few routes in the functions directory that stopped working because of this change.

Is it possible to factor those in? Or have a way of supplementing the 'includes' array somehow?

[schummar]

That is true. The manual work around right now: You can define your own _routes.json.
https://docs.astro.build/de/guides/integrations-guide/cloudflare/#custom-_routesjson

To properly address the issue I see multiple possibilities:

1. Make the strategy configurable. As I mentioned, at the moment the strategy that produces less entries will be chosen. We could easily allow users to override it in the integrations config - e.g. routes: { strategy: 'exclude' | 'include' | 'auto' }

2. Allow modifying the routes in the config. E.g. routes: { include: ['/myCustomFunction'], exclude: [] } which could be merged with the automatically calculated routes.

3. Traverse the functions directory and include the routes automatically. I haven't done much with custom routes so far so I can't say how complex that can become. I imagine picking up simple routes like functions/myFunction.js would be easy, while handling dynamic routes there could be a problem.

The first two options are straight forward to implement but add to the config and docs complexity, so the team should voice their opinion about it. The third option is the most powerful but also most complex to implement. I suppose the recommended way to do functions is to write Astro endpoints and let the functions be generated (in which case everything works fine), so the third option might not be worth it.

My recommendation is implementing 1 and 2. Opinions?

[vvashisht]

Yes, that is what I ended up doing indeed :)

Cloudflare's 100 rule constraint outweighs the hassle of manual route configuration. So, I do think that this PR serves a critical role.

On the other hand, if the current implementation doesn't evolve, anyone using the 'directory' mode to implement routes/pages/middleware outside astro's scope can not benefit from optimized automatic routing config.

Also, I might not be the only one facing this problem right now.

In my opinion, as a first step, the documentation should mention that opting for 'directory' mode necessitates manual routing config, if one intends to put any route in the functions directory. The possible points for this addition can be at one or both locations:
https://docs.astro.build/en/guides/integrations-guide/cloudflare/#mode
https://docs.astro.build/en/guides/integrations-guide/cloudflare/#custom-_routesjson

Further, a build log notice can be added to communicate that in absence of manual config, 'functions' routes will be ignored.

---

As for the route generation logic, I agree with your proposal. I think option 3 is both an overkill and a bug trap.

It would be ideal to implement both 1 and 2, but, and this is just my opinion, option 2 alone could suffice in a pinch.

Having said that, for option 2, merging of user defined config needs a bit of consideration regarding conflict resolution.

[vvashisht]

@ematipico I understand that the team must be busy on multiple fronts. But if you could please spare a quick opinion:

Should this side effect be fixed? If so, should I create a separate issue for it?

[schummar]

I am back from vacation and have created a pr with a proposal: #8459