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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[next] support middleware-manifest v2 #8319

Merged
merged 15 commits into from Aug 25, 2022

Conversation

nkzawa
Copy link
Contributor

@nkzawa nkzawa commented Aug 4, 2022

Related Issues

As the title, support a new version of middleware-manifest of next.js that is going to be added in vercel/next.js#39257

馃搵 Checklist

Tests

  • The code changed/added as part of this PR has been covered with tests
  • All tests pass locally with yarn test-unit

Code Review

  • This PR has a concise title and thorough description useful to a reviewer
  • Issue from task tracker has a link to this PR

Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

I'm seeing the below error while testing, we may want to clean up the error as it's not exactly clear which route was incorrect and the expected values.

21:55:12.935 | Created all serverless functions in: 46.001ms
21:55:12.967 | Collected static files (public/, static/, .next/static): 2.554ms
21:55:13.161 | Build Completed in /vercel/output [24s]
21:55:13.646 | Error: Mapping middleware not found. Maybe you provided a wrong middlewarePath? Available paths: middleware

Copy link
Member

@feugy feugy left a comment

Choose a reason for hiding this comment

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

Besides JJ's request for better error handling, no extra requests.

packages/next/src/utils.ts Show resolved Hide resolved
packages/next/src/utils.ts Show resolved Hide resolved
@nkzawa
Copy link
Contributor Author

nkzawa commented Aug 12, 2022

The error @ijjk mentioned should have already been fixed by https://github.com/vercel/api/pull/13624

Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

The changes here look good, will hold off on approving until the e2e deploy tests are passing on the Next.js PR though

Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

Thanks!

@kodiakhq kodiakhq bot merged commit ead1e41 into main Aug 25, 2022
@kodiakhq kodiakhq bot deleted the next-support-v2-middleware-manifest branch August 25, 2022 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: next semver: patch PR contains bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants