-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
Reduce amount of rules in routes json file #394
Reduce amount of rules in routes json file #394
Conversation
🦋 Changeset detectedLatest commit: 342d0a3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
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 |
@veitbjarsch Thank you for this contribution, I'll check it later to see if it works with all requirements outlined in this issue #374 |
I have to check this internally, but I think we can't tell this upfront, either in all cases or in specific cases.
That's a good addition, we need to see how it handles the routes if you have all static in one folder and then one dynamic route in a nested subfolder. I'll take a look at your testcase. |
@alexanderniebuhr Thanks for your fast response.
The code compares both includes and excludes in 2 ways. This means if the same folder (partial path) is present in both PathTrie objects, it will leave them untouched. Only if a folder is not present in the other PathTrie object we can savely assume that all children belongs to the current PathTrie. Only in this case we add the wildcard flag for this folder. This way we can reduce the amount of rules in our project by more than 50% automatically, without the need to create and handle the routes.jsnon manually. |
Sounds good, I did a quick test and it seems to be working with all the cases. I'll give it a proper review later :) |
Thanks |
@veitbjarsch can you add an changeset with |
@alexanderniebuhr added the changeset. |
@alexanderniebuhr Hey, is it planned to merge this PR into the main branch? |
Changes
Cloudflare has a limit of 100 includes/exclude rules. In many applications this amount is reached quite fast.
This PR add the functionality to compare include and exclude rules and reduce the amount of rules wherever possible.
Therefore it:
Testing
Tested in out own project using cloudflare as well wrote a unit test here.
Docs