fix(shims): add runtime default export to next/app#1204
Conversation
Pages Router fixtures using the canonical `pages/_app.js` pattern
(`import App from 'next/app'; export default class MyApp extends App`)
currently fail to build under vinext with:
[MISSING_EXPORT] "default" is not exported by
"node_modules/.../vinext/dist/shims/app.js"
The existing shim only re-exported a type as default. This adds a real
runtime class component matching Next.js's behaviour:
- `App` extends React.Component, renders `<Component {...pageProps} />`.
- `App.getInitialProps(appContext)` returns `{ pageProps }`, invoking the
wrapped page's `getInitialProps` if defined.
- `App.origGetInitialProps` is preserved alongside `getInitialProps`.
- AppProps, AppContext, AppInitialProps types are exported.
Ported from:
https://github.com/vercel/next.js/blob/canary/packages/next/src/pages/_app.tsx
This unblocks 51 build failures in the Next.js deploy suite that all
trace back to the missing default export.
|
/bigbonk review |
commit: |
|
There was a problem hiding this comment.
Looks good. The shim faithfully ports the essential behavior from Next.js _app.tsx — class shape, origGetInitialProps/getInitialProps statics, and the render() delegation all match upstream.
Tests are thorough: class component shape assertion, renderToStaticMarkup verification of render output, getInitialProps delegation with and without Component.getInitialProps.
The intentional omissions (dev-mode validation from loadGetInitialProps, AppType/AppPropsType re-exports) are well-documented and reasonable for the current scope.
Minor gap (not blocking): next-shims.d.ts lines 506-512 still declare the old minimal AppProps-only shape without the default class export, AppContext, or AppInitialProps. Since the shim map in index.ts resolves to the source .tsx file directly, this does not affect builds — but external .d.ts consumers would not see the new types. Fine as a follow-up.
One nit inline on null-coercion behavior.
| pageProps = await Component.getInitialProps(ctx); | ||
| if (pageProps == null) { | ||
| pageProps = {}; | ||
| } |
There was a problem hiding this comment.
nit: Next.js's loadGetInitialProps throws here when the result is null/undefined ("should resolve to an object. But found \"${props}\" instead."), while this silently coerces to {}. The PR documents this as intentional and it's fine for the current scope — well-behaved getInitialProps implementations won't return nullish. Just flagging for awareness: if a future deploy-suite failure shows a page rendering with empty props when it should have errored, this is the divergence point.
|
Review posted successfully. Here's a summary of what I reviewed and the outcome: Verdict: APPROVED The PR adds a runtime default export to the
One nit flagged (inline comment on One minor gap noted (not blocking): |
…l coercion Address two nits from the PR #1204 bonk review: 1. `next-shims.d.ts` still declared the old minimal `AppProps`-only shape. External `.d.ts` consumers (editor, type checks against the published package) couldn't see the new runtime class, `AppContext`, or `AppInitialProps`. Update the `declare module 'next/app'` block to declare the full surface added in the shim: - default `class App<P, CP, S>` extending `React.Component` with `static origGetInitialProps` / `static getInitialProps` and a `render(): React.ReactNode` method - `AppContext` and `AppInitialProps<P>` type aliases - Expanded `AppProps<P>` shape (router, __N_SSG/__N_SSP flags) 2. `appGetInitialProps` silently coerces a null/undefined return from `Component.getInitialProps` to `{}`. Next.js's `loadGetInitialProps` throws in that case. Add an inline comment at the coercion site noting the divergence, linking to the upstream source, and giving future maintainers a bread crumb when they're debugging Pages Router pages that render with mysteriously empty props.
|
/bigbonk review |
There was a problem hiding this comment.
Clean, well-scoped fix. The shim faithfully replicates the essential runtime behavior from Next.js's _app.tsx — class shape, getInitialProps/origGetInitialProps statics, and render() delegation all match upstream. The type declarations in next-shims.d.ts were updated to reflect the new default class export, AppContext, and AppInitialProps. Tests are solid.
Ship it.
|
Review posted: APPROVED The PR is clean and well-scoped. Here's my assessment: What it does: Replaces the type-only default export in What I verified:
|
Summary
Adds a runtime
defaultexport topackages/vinext/src/shims/app.ts(now.tsx) so Pages Router fixtures that follow the canonicalpages/_app.jspattern can build under vinext.The problem
Pages Router code that does:
currently fails to build under vinext with:
The existing shim only re-exported a type as default (
export type { AppProps as default }), with no runtime value.This is the single highest-leverage shim fix in the Next.js deploy suite: 51 build failures all trace back to this missing default export.
What I ported
From
packages/next/src/pages/_app.tsx(Next.js canary):AppextendsReact.Component<P & AppProps<CP>, S>and renders<Component {...pageProps} />.App.getInitialProps(appContext)returns{ pageProps }— invokes the wrapped page'sgetInitialPropsif defined, otherwise returns{ pageProps: {} }.App.origGetInitialPropsis preserved alongsidegetInitialProps(Next.js userland code occasionally introspects this).AppProps,AppContext, andAppInitialPropstypes are exported.The original Next.js implementation delegates to
loadGetInitialPropsinshared/lib/utils.ts. For the canonical_apppattern, the only relevant behaviour is thepagePropsextraction, which is replicated inline; the dev-only validation (getInitialPropsas an instance method, response-sent check, empty-object warning) is intentionally omitted — see "Intentional omissions" below.Tests
tests/shims.test.tsgains anext/app shimdescribewith four cases:prototype.render, instances areReact.Componentinstances).App.render()produces<Component {...pageProps} />(asserted viarenderToStaticMarkup).App.getInitialPropsis a function, returns{ pageProps: <result> }whenComponent.getInitialPropsis defined, andorigGetInitialPropsis preserved.App.getInitialPropsreturns{ pageProps: {} }whenComponenthas nogetInitialProps.Run with:
vp test run tests/shims.test.ts -t "next/app shim"— all 4 pass, full file (870 tests) green.Intentional omissions
These bits of Next.js's
_app.tsxwere intentionally left out because they are not exercised by Pages Router test fixtures and would expand the shim surface beyond what the failing deploy-suite builds need:loadGetInitialPropsdev-mode validation — Next.js throws ifgetInitialPropsis defined as an instance method, warns on empty objects, and checksisResSent(res). Userland code callingApp.getInitialProps(appContext)doesn't depend on these. If the deploy suite later catches a fixture that relies on them, they're easy to port over.AppType,AppPropsType,AppTreeTypeas re-exports — the typescript test suite importsAppTypefromnext/app, but no JS fixture does. Adding these is a follow-up if the typing parity gap matters.NextPageContext/Routertyping —ctxandrouterare typed asanyhere, matching Next.js's permissive_app.tsxgenerics (<P = any>).Verification
vp test run tests/shims.test.ts— 870/870 passvp check— formatting, lint, type checks cleanRelated
This is the first of a series of focused shim fixes against the Next.js deploy suite. Other shim-level parity gaps are separate PRs.