Skip to content
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

feat!: correctly interpret /**/* paths #440

Closed
wants to merge 3 commits into from
Closed

feat!: correctly interpret /**/* paths #440

wants to merge 3 commits into from

Conversation

Skn0tt
Copy link
Contributor

@Skn0tt Skn0tt commented Jul 25, 2023

This came up over in this forums post: https://answers.netlify.com/t/please-clarify-documentation-for-edge-functions-excludedpath/97443

Given the path /**/*.html, i'd expect it to match both /foo.html and /sub/foo.html. With our current handling, only /sub/foo.html is matched however - i've demonstrated this in a test added in the first commit of this PR.

After reading into this, it turns out that the special meaning of ** was introduced "recently" in 2010 and is called globstar. We don't have it enabled for edge functions currently, which means that /**/*.html is equivalent to /*/*.html. This probably comes at a surprise to our customers.

This PR enables the globstar option. This is a breaking change to what we implemented, and i'm unsure what's the best way of rolling this out. Here's a matrix for how it'd change behaviour:

matched paths, currently with globstar
/*.html /foo.html, /sub/foo.html /foo.html
/**/*.html /sub/foo.html /foo.html, /sub/foo.html

This would essentially break sites that have an edge function at /*, so i'm really unsure if this is a change we can make now 🤔 Let's have a discussion on that.

@Skn0tt Skn0tt requested a review from a team as a code owner July 25, 2023 11:58
@Skn0tt
Copy link
Contributor Author

Skn0tt commented Jul 25, 2023

closing in favor of #392

@Skn0tt Skn0tt closed this Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant