Skip to content

Enhance header filtering in getHeadersApplicableToAllResources function to exclude falsy values#588

Merged
vejja merged 3 commits intoBaroshem:mainfrom
ivanvakulov:origin/issue587
Dec 30, 2024
Merged

Enhance header filtering in getHeadersApplicableToAllResources function to exclude falsy values#588
vejja merged 3 commits intoBaroshem:mainfrom
ivanvakulov:origin/issue587

Conversation

@ivanvakulov
Copy link
Copy Markdown
Contributor

@ivanvakulov ivanvakulov commented Dec 17, 2024 •
edited by vejja
Loading

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

This pull request includes a small change to the src/utils/headers.ts file. The change ensures that only headers with non-falsy values are included in the result.

  • src/utils/headers.ts: Added a filter to remove headers with falsy values in the getHeadersApplicableToAllResources function.

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes (if not applicable, please state why)

@ivanvakulov ivanvakulov mentioned this pull request Dec 17, 2024
Closed
vejja
vejja requested changes Dec 19, 2024
View reviewed changes
@vejja
Copy link
Copy Markdown
Collaborator

vejja commented Dec 19, 2024 •
edited
Loading

@ivanvakulov Thank you very much for this PR
Would you be able to add unit tests to make sure that the functionality works as expected, including for falsy values such as undefined?
I think they probably belong to /test/perRoute
Just want to make sure that setting false somewhere in the hierarchy of route rules won't impact another sub route somewhere

@vercel
Copy link
Copy Markdown

vercel bot commented Dec 19, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
security ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 19, 2024 7:42pm

@ivanvakulov
Copy link
Copy Markdown
Contributor Author

ivanvakulov commented Dec 19, 2024

@vejja Thank you for the review. I've added new tests for publicAssets to ensure there are no empty headers and that headers are correctly set according to the routeRules.

@vejja
Copy link
Copy Markdown
Collaborator

vejja commented Dec 20, 2024

@Baroshem LGTM

Baroshem
Baroshem approved these changes Dec 23, 2024
View reviewed changes
Copy link
Copy Markdown
Owner

@Baroshem Baroshem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work @ivanvakulov

Thanks @vejja for checking it!

@vejja vejja merged commit 9f17263 into Baroshem:main Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vejja vejja vejja approved these changes

@Baroshem Baroshem Baroshem approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ivanvakulov @vejja @Baroshem