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

Jump to bottom

Throw NoFallbackError instead of returning #10793

Merged
merged 5 commits into from Mar 3, 2020
Merged

Throw NoFallbackError instead of returning #10793

merged 5 commits into from Mar 3, 2020

Conversation

devknoll
Copy link
Contributor

@devknoll devknoll commented Mar 2, 2020

The false variation can't cleanly be modeled in a (req, res) => Promise<void> universe, so lets change it to throw an error for now. We can probably remove this in the future.

@ijjk
Copy link
Member

ijjk commented Mar 2, 2020

Stats from current PR

Default Server Mode
General Overall increase ⚠️
zeit/next.js canary devknoll/next.js add-next-request-response Change
buildDuration 10.9s 10.9s -6ms
nodeModulesSize 56.3 MB 56.3 MB ⚠️ +200 B
Client Bundles (main, webpack, commons)
zeit/next.js canary devknoll/next.js add-next-request-response Change
main-HASH.js gzip 5.76 kB 5.76 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
de003c3a9d30..67cb.js gzip 9.69 kB 9.69 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 55.3 kB 55.3 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary devknoll/next.js add-next-request-response Change
main-HASH.module.js gzip 4.79 kB 4.79 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
de003c3a9d30..dule.js gzip 6.64 kB 6.64 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 51.3 kB 51.3 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary devknoll/next.js add-next-request-response Change
polyfills-HASH.js gzip 18.9 kB 18.9 kB ✓
Overall change 18.9 kB 18.9 kB ✓
Client Pages
zeit/next.js canary devknoll/next.js add-next-request-response Change
_app.js gzip 1.07 kB 1.07 kB ✓
_error.js gzip 2.98 kB 2.98 kB ✓
hooks.js gzip 664 B 664 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 1.89 kB 1.89 kB ✓
routerDirect.js gzip 279 B 279 B ✓
withRouter.js gzip 278 B 278 B ✓
Overall change 7.38 kB 7.38 kB ✓
Client Pages Modern
zeit/next.js canary devknoll/next.js add-next-request-response Change
_app.module.js gzip 589 B 589 B ✓
_error.module.js gzip 2.08 kB 2.08 kB ✓
hooks.module.js gzip 370 B 370 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 1.48 kB 1.48 kB ✓
routerDirect..dule.js gzip 271 B 271 B ✓
withRouter.m..dule.js gzip 270 B 270 B ✓
Overall change 5.27 kB 5.27 kB ✓
Client Build Manifests
zeit/next.js canary devknoll/next.js add-next-request-response Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Rendered Page Sizes
zeit/next.js canary devknoll/next.js add-next-request-response Change
index.html gzip 936 B 936 B ✓
link.html gzip 946 B 946 B ✓
withRouter.html gzip 934 B 934 B ✓
Overall change 2.82 kB 2.82 kB ✓
Serverless Mode (Increase detected ⚠️)
General Overall increase ⚠️
zeit/next.js canary devknoll/next.js add-next-request-response Change
buildDuration 11.6s 11.6s -55ms
nodeModulesSize 56.3 MB 56.3 MB ⚠️ +200 B
Client Bundles (main, webpack, commons)
zeit/next.js canary devknoll/next.js add-next-request-response Change
main-HASH.js gzip 5.76 kB 5.76 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
de003c3a9d30..67cb.js gzip 9.69 kB 9.69 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 55.3 kB 55.3 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary devknoll/next.js add-next-request-response Change
main-HASH.module.js gzip 4.79 kB 4.79 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
de003c3a9d30..dule.js gzip 6.64 kB 6.64 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 51.3 kB 51.3 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary devknoll/next.js add-next-request-response Change
polyfills-HASH.js gzip 18.9 kB 18.9 kB ✓
Overall change 18.9 kB 18.9 kB ✓
Client Pages
zeit/next.js canary devknoll/next.js add-next-request-response Change
_app.js gzip 1.07 kB 1.07 kB ✓
_error.js gzip 2.98 kB 2.98 kB ✓
hooks.js gzip 664 B 664 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 1.89 kB 1.89 kB ✓
routerDirect.js gzip 279 B 279 B ✓
withRouter.js gzip 278 B 278 B ✓
Overall change 7.38 kB 7.38 kB ✓
Client Pages Modern
zeit/next.js canary devknoll/next.js add-next-request-response Change
_app.module.js gzip 589 B 589 B ✓
_error.module.js gzip 2.08 kB 2.08 kB ✓
hooks.module.js gzip 370 B 370 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 1.48 kB 1.48 kB ✓
routerDirect..dule.js gzip 271 B 271 B ✓
withRouter.m..dule.js gzip 270 B 270 B ✓
Overall change 5.27 kB 5.27 kB ✓
Client Build Manifests
zeit/next.js canary devknoll/next.js add-next-request-response Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Serverless bundles Overall increase ⚠️
zeit/next.js canary devknoll/next.js add-next-request-response Change
_error.js gzip 292 kB 292 kB -218 B
404.html gzip 1.34 kB 1.34 kB ✓
hooks.html gzip 975 B 975 B ✓
index.js gzip 292 kB 292 kB ⚠️ +533 B
link.js gzip 299 kB 299 kB ⚠️ +27 B
routerDirect.js gzip 298 kB 298 kB -165 B
withRouter.js gzip 299 kB 299 kB -3 B
Overall change 1.48 MB 1.48 MB ⚠️ +174 B

Commit: 500db97

@devknoll devknoll marked this pull request as ready for review March 2, 2020 19:04
ijjk
ijjk approved these changes Mar 3, 2020
View reviewed changes
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.

LGTM, will leave for @Timer to also review

Timer
Timer requested changes Mar 3, 2020
View reviewed changes
Copy link
Member

@Timer Timer left a comment •
edited

Choose a reason for hiding this comment

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

While I'm not a fan of using errors as control-flow (and TypeScript reasons), I understand the [temporary] necessity of this change.

packages/next/next-server/server/next-server.ts Show resolved Hide resolved
@devknoll devknoll requested a review from Timer March 3, 2020 18:15
@ijjk
Copy link
Member

ijjk commented Mar 3, 2020

Stats from current PR

Default Server Mode
General Overall increase ⚠️
zeit/next.js canary devknoll/next.js add-next-request-response Change
buildDuration 10.7s 10.6s -139ms
nodeModulesSize 56.3 MB 56.3 MB ⚠️ +658 B
Client Bundles (main, webpack, commons)
zeit/next.js canary devknoll/next.js add-next-request-response Change
main-HASH.js gzip 5.76 kB 5.76 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
de003c3a9d30..67cb.js gzip 9.69 kB 9.69 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 55.3 kB 55.3 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary devknoll/next.js add-next-request-response Change
main-HASH.module.js gzip 4.79 kB 4.79 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
de003c3a9d30..dule.js gzip 6.64 kB 6.64 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 51.3 kB 51.3 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary devknoll/next.js add-next-request-response Change
polyfills-HASH.js gzip 18.9 kB 18.9 kB ✓
Overall change 18.9 kB 18.9 kB ✓
Client Pages
zeit/next.js canary devknoll/next.js add-next-request-response Change
_app.js gzip 1.07 kB 1.07 kB ✓
_error.js gzip 2.98 kB 2.98 kB ✓
hooks.js gzip 664 B 664 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 1.89 kB 1.89 kB ✓
routerDirect.js gzip 279 B 279 B ✓
withRouter.js gzip 278 B 278 B ✓
Overall change 7.38 kB 7.38 kB ✓
Client Pages Modern
zeit/next.js canary devknoll/next.js add-next-request-response Change
_app.module.js gzip 589 B 589 B ✓
_error.module.js gzip 2.08 kB 2.08 kB ✓
hooks.module.js gzip 370 B 370 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 1.48 kB 1.48 kB ✓
routerDirect..dule.js gzip 271 B 271 B ✓
withRouter.m..dule.js gzip 270 B 270 B ✓
Overall change 5.27 kB 5.27 kB ✓
Client Build Manifests
zeit/next.js canary devknoll/next.js add-next-request-response Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Rendered Page Sizes
zeit/next.js canary devknoll/next.js add-next-request-response Change
index.html gzip 936 B 936 B ✓
link.html gzip 946 B 946 B ✓
withRouter.html gzip 934 B 934 B ✓
Overall change 2.82 kB 2.82 kB ✓
Serverless Mode (Increase detected ⚠️)
General Overall increase ⚠️
zeit/next.js canary devknoll/next.js add-next-request-response Change
buildDuration 11.3s 11.5s ⚠️ +145ms
nodeModulesSize 56.3 MB 56.3 MB ⚠️ +658 B
Client Bundles (main, webpack, commons)
zeit/next.js canary devknoll/next.js add-next-request-response Change
main-HASH.js gzip 5.76 kB 5.76 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
de003c3a9d30..67cb.js gzip 9.69 kB 9.69 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 55.3 kB 55.3 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary devknoll/next.js add-next-request-response Change
main-HASH.module.js gzip 4.79 kB 4.79 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
de003c3a9d30..dule.js gzip 6.64 kB 6.64 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 51.3 kB 51.3 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary devknoll/next.js add-next-request-response Change
polyfills-HASH.js gzip 18.9 kB 18.9 kB ✓
Overall change 18.9 kB 18.9 kB ✓
Client Pages
zeit/next.js canary devknoll/next.js add-next-request-response Change
_app.js gzip 1.07 kB 1.07 kB ✓
_error.js gzip 2.98 kB 2.98 kB ✓
hooks.js gzip 664 B 664 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 1.89 kB 1.89 kB ✓
routerDirect.js gzip 279 B 279 B ✓
withRouter.js gzip 278 B 278 B ✓
Overall change 7.38 kB 7.38 kB ✓
Client Pages Modern
zeit/next.js canary devknoll/next.js add-next-request-response Change
_app.module.js gzip 589 B 589 B ✓
_error.module.js gzip 2.08 kB 2.08 kB ✓
hooks.module.js gzip 370 B 370 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 1.48 kB 1.48 kB ✓
routerDirect..dule.js gzip 271 B 271 B ✓
withRouter.m..dule.js gzip 270 B 270 B ✓
Overall change 5.27 kB 5.27 kB ✓
Client Build Manifests
zeit/next.js canary devknoll/next.js add-next-request-response Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Serverless bundles Overall increase ⚠️
zeit/next.js canary devknoll/next.js add-next-request-response Change
_error.js gzip 292 kB 292 kB -534 B
404.html gzip 1.34 kB 1.34 kB ✓
hooks.html gzip 975 B 975 B ✓
index.js gzip 292 kB 292 kB -215 B
link.js gzip 299 kB 300 kB ⚠️ +888 B
routerDirect.js gzip 299 kB 299 kB ⚠️ +43 B
withRouter.js gzip 298 kB 298 kB -173 B
Overall change 1.48 MB 1.48 MB ⚠️ +9 B

@Timer Timer added this to the 9.3.0 milestone Mar 3, 2020
@Timer Timer merged commit 3ca022a into vercel:canary Mar 3, 2020
@devknoll devknoll deleted the add-next-request-response branch March 3, 2020 18:33
@snyk-bot snyk-bot mentioned this pull request Apr 9, 2020
Open
@vercel vercel locked as resolved and limited conversation to collaborators Jan 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Timer Timer Timer approved these changes

@ijjk ijjk ijjk approved these changes

@lfades lfades Awaiting requested review from lfades

@timneutkens timneutkens Awaiting requested review from timneutkens timneutkens is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
9.3.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@devknoll @ijjk @Timer