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

Fix regex page matcher for Next.js middlewares version 1 #5496

Merged
merged 14 commits into from
May 3, 2023

Conversation

leoortizz
Copy link
Member

Description

Projects using the first version of the Next.js middleware were failing to deploy due to a difference in middleware-manifest.json. The first version was in Next.js until v12.1.6. Version 2 was introduced in Next.js 12.2.0. This PR handles the middleware page matchers for version 1.

Scenarios Tested

Sample Commands

src/frameworks/next/index.ts Outdated Show resolved Hide resolved
@jamesdaniels
Copy link
Member

Are there any other Next 12 gotchas? E.g, rewrites/redirects/headers?

@leoortizz
Copy link
Member Author

Are there any other Next 12 gotchas? E.g, rewrites/redirects/headers?

I can check but I don't think so. Middlewares was still in beta before Next.js 12.2 so that was the main thing we're dealing with that has changed

@jamesdaniels
Copy link
Member

Add changelog

@codecov-commenter
Copy link

codecov-commenter commented Feb 10, 2023

Codecov Report

Patch coverage: 90.90% and project coverage change: +0.02 🎉

Comparison is base (155d80d) 55.20% compared to head (0dc2904) 55.23%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5496      +/-   ##
==========================================
+ Coverage   55.20%   55.23%   +0.02%     
==========================================
  Files         328      328              
  Lines       22410    22415       +5     
  Branches     4573     4574       +1     
==========================================
+ Hits        12372    12381       +9     
+ Misses       8936     8932       -4     
  Partials     1102     1102              
Impacted Files Coverage Δ
src/frameworks/next/index.ts 14.71% <66.66%> (-0.24%) ⬇️
src/frameworks/next/utils.ts 90.21% <100.00%> (+0.93%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@TheIronDev TheIronDev left a comment

Choose a reason for hiding this comment

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

Please write a test to "prove" this addresses the mismatch middleware issue.

@leoortizz leoortizz requested review from TheIronDev and jamesdaniels and removed request for jamesdaniels March 16, 2023 21:30
@leoortizz leoortizz enabled auto-merge (squash) May 2, 2023 00:42
@jamesdaniels jamesdaniels dismissed TheIronDev’s stale review May 3, 2023 14:27

Feedback addressed.

@leoortizz leoortizz merged commit 4d6da7e into firebase:master May 3, 2023
33 of 34 checks passed
ProfHercules pushed a commit to ProfHercules/firebase-tools that referenced this pull request May 5, 2023
* fix regex matcher for middleware version 1

* MiddlewareManifest type

* changelog

* get middleware matcher regexes as util + unit tests

* prettier in changelog
tonyjhuang pushed a commit that referenced this pull request May 22, 2023
* fix regex matcher for middleware version 1

* MiddlewareManifest type

* changelog

* get middleware matcher regexes as util + unit tests

* prettier in changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants