feat(next-pwa): modernize for Next.js 16 and React 19#38
Closed
riceharvest wants to merge 74 commits intomainfrom
Closed
feat(next-pwa): modernize for Next.js 16 and React 19#38riceharvest wants to merge 74 commits intomainfrom
riceharvest wants to merge 74 commits intomainfrom
Conversation
- Upgraded multiple packages to modern standards (Next.js, Next-auth, PWA, SEO). - Added new utility packages: critters, next-circuit-breaker, next-csrf, next-images, next-json-ld. - Integrated Changesets for versioning. - Updated CI/CD workflows and linting configurations. - Fixed numerous linting and type-checking issues across the monorepo.
- Remove legacy NextAuth adapters and resolve workspace version conflicts - Clean up test warning noise and fix tsconfig/jest setups for next-auth - Update Workbox/Terser dependencies in next-pwa to align with workspace - Synchronize root lockfile to reflect nested package resolutions
Fixes `JWT_AUTO_GENERATED_SIGNING_KEY` and `JWT_AUTO_GENERATED_ENCRYPTION_KEY` warnings properly by supplying JWKs directly in the test suite rather than mocking the logger.
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoModernize monorepo for Next.js 16 and React 19 with enhanced features and test migrations
WalkthroughsDescription• **Modernized Next.js and React support**: Upgraded to Next.js 16 and React 19 with comprehensive compatibility updates across multiple packages • **Test framework migrations**: Migrated test suites from Jest/vitest to Node.js native node:test module and updated vitest usage patterns • **New MDX functionality**: Added complete MDX support with node retrieval, configuration loading, file discovery, path generation, and table of contents utilities • **Enhanced session management**: Refactored session decoration with Web API support alongside Node.js APIs, improved type safety, and added time parsing utilities • **CSRF improvements**: Enhanced token validation with multiple source support, separated cookie options for secrets vs tokens, and added App Router verification • **Authentication examples**: Added comprehensive react-query-auth example with mock API server, database, and authentication flows • **OAuth modernization**: Removed external oauth package dependency with native fetch-based implementation • **Bug fixes**: Improved CSS inlining robustness in critters, fixed container detection, and enhanced error handling • **Build configuration updates**: Modernized tsup and vitest configurations across packages • **Type definitions**: Added TypeScript declarations for PWA, react-virtualized, and MDX client modules • **Test simplifications**: Reduced test file sizes by removing redundant coverage while maintaining core functionality validation Diagramflowchart LR
A["Next.js 16<br/>React 19"] -->|"Upgrade"| B["Core Packages"]
B -->|"Migrate Tests"| C["Node.js native test<br/>Vitest updates"]
B -->|"Add Features"| D["MDX Support<br/>Session Web API<br/>CSRF App Router"]
B -->|"Refactor"| E["OAuth native impl<br/>Type definitions"]
D -->|"Enable"| F["New Examples<br/>react-query-auth"]
E -->|"Improve"| G["Bug Fixes<br/>Build Config"]
File Changes1. packages/next-images/test/index.test.ts
|
Code Review by Qodo
1. next-mdx entrypoint not src/index.ts
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d17ff87945
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+13
to
+16
| return { | ||
| getOAuthAccessToken: (code, codeVerifier) => getOAuth2AccessToken(code, provider, codeVerifier), | ||
| get: (accessToken, results) => getOAuth2(provider, accessToken, results) | ||
| } |
There was a problem hiding this comment.
The new OAuth2 branch now returns a plain object with only getOAuthAccessToken and get, but the callback flow still unconditionally calls client.useAuthorizationHeaderforGET(...) before exchanging the code (packages/next-auth/src/server/lib/oauth/callback.js). With this interface mismatch, every OAuth2 sign-in callback throws a TypeError at runtime and authentication cannot complete for providers like Google/GitHub.
Useful? React with 👍 / 👎.
Comment on lines
+225
to
+228
| async getOAuthRequestToken(params = {}) { | ||
| // Implement OAuth 1.0a request token logic | ||
| throw new Error("OAuth 1.0a is not yet fully implemented in the native client. Please use OAuth 2.0 or contact maintainers.") | ||
| } |
There was a problem hiding this comment.
The OAuth1 client methods now throw not yet fully implemented errors, so OAuth1 providers can no longer authenticate. This is a hard regression for configured OAuth1 providers (for example Twitter version: "1.0A"), because the callback path calls getOAuthRequestToken/getOAuthAccessToken/get and now always fails.
Useful? React with 👍 / 👎.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+6
to
+8
| "source": "src/server.ts", | ||
| "main": "dist/server.js", | ||
| "types": "dist/server.d.ts", |
There was a problem hiding this comment.
1. next-mdx entrypoint not src/index.ts 📘 Rule violation ⛯ Reliability
The new @opensourceframework/next-mdx package declares source as src/server.ts, which makes the package’s public entrypoint deviate from the required src/index.ts convention. This breaks the monorepo’s standard entrypoint expectations for tooling and consumers.
Agent Prompt
## Issue description
`packages/next-mdx` does not follow the monorepo convention that the public package entrypoint is `src/index.ts`.
## Issue Context
The package metadata currently declares `source` as `src/server.ts`, implying the package entrypoint is not `src/index.ts`.
## Fix Focus Areas
- packages/next-mdx/package.json[6-8]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+59
to
85
| test("should validate JSON-LD against a minimal WebPage schema", async ({ | ||
| page, | ||
| }) => { | ||
| await page.goto(PAGE_URL); | ||
| const scriptHandle = await page.waitForSelector(SCRIPT_SELECTOR); | ||
| const scriptContent = await scriptHandle.innerHTML(); | ||
| const jsonData = JSON.parse(scriptContent); | ||
| const ajv = new Ajv(); | ||
| const validate = ajv.compile({ | ||
| type: "object", | ||
| required: ["@context", "@type", "name", "description", "url"], | ||
| properties: { | ||
| "@context": { const: "https://schema.org" }, | ||
| "@type": { const: "WebPage" }, | ||
| name: { type: "string", minLength: 1 }, | ||
| description: { type: "string", minLength: 1 }, | ||
| url: { type: "string", minLength: 1 }, | ||
| }, | ||
| additionalProperties: true, | ||
| }); | ||
|
|
||
| // Placeholder for Ajv validation | ||
| // const ajv = new Ajv(); | ||
| // const webPageSchema = require('../../schemas/webpage.schema.json'); // You'll create this | ||
| // const validate = ajv.compile(webPageSchema); | ||
| // const valid = validate(jsonData); | ||
| // if (!valid) console.error(validate.errors); | ||
| // expect(valid, `JSON-LD should be valid according to WebPage schema. Errors: ${JSON.stringify(validate.errors)}`).toBe(true); | ||
| expect(jsonData).toBeTruthy(); // Keep a basic assertion for now | ||
| const valid = validate(jsonData); | ||
| expect( | ||
| valid, | ||
| `JSON-LD should match schema. Errors: ${JSON.stringify(validate.errors)}` | ||
| ).toBe(true); | ||
| }); |
There was a problem hiding this comment.
2. jsonldscript test file misnamed 📘 Rule violation ⛯ Reliability
A test is being added/extended in jsonLdScript.e2e.spec.ts, which does not follow the required *.test.ts / *.test.tsx naming convention. This can reduce test discoverability/consistency with repo test tooling conventions.
Agent Prompt
## Issue description
A test file being modified/extended is named `*.spec.ts`, but the repo convention requires `*.test.ts` or `*.test.tsx`.
## Issue Context
This change adds additional E2E test behavior in `jsonLdScript.e2e.spec.ts`.
## Fix Focus Areas
- packages/next-seo/tests/e2e/jsonLdScript.e2e.spec.ts[59-85]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1
to
+10
| import { defineConfig } from "vitest/config" | ||
|
|
||
| export default defineConfig({ | ||
| test: { | ||
| globals: true, | ||
| env: { | ||
| NEXTAUTH_URL: "http://localhost:3000/api/auth", | ||
| }, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
3. vitest.config.js not prettier-formatted 📘 Rule violation ✓ Correctness
The new vitest.config.js uses double quotes and omits semicolons, which conflicts with the repo’s Prettier standards. This introduces inconsistent formatting and can cause noisy diffs in future changes.
Agent Prompt
## Issue description
`vitest.config.js` does not match the repo Prettier rules (e.g., double quotes and missing semicolons).
## Issue Context
This is a newly added config file; formatting should match the repository source-of-truth Prettier style to avoid inconsistency.
## Fix Focus Areas
- vitest.config.js[1-10]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1
to
19
| // @vitest-environment happy-dom | ||
| import React from "react" | ||
| import { http, HttpResponse } from "msw" | ||
| import { useState } from "react" | ||
| import userEvent from "@testing-library/user-event" | ||
| import { render, screen, waitFor } from "@testing-library/react" | ||
| import { server, mockProviders } from "./helpers/mocks" | ||
| import { getProviders } from ".." | ||
| import logger from "../../lib/logger" | ||
| import { rest } from "msw" | ||
|
|
||
| jest.mock("../../lib/logger", () => ({ | ||
| vi.mock("../../lib/logger", () => ({ | ||
| __esModule: true, | ||
| default: { | ||
| warn: jest.fn(), | ||
| debug: jest.fn(), | ||
| error: jest.fn(), | ||
| warn: vi.fn(), | ||
| debug: vi.fn(), | ||
| error: vi.fn(), | ||
| }, | ||
| proxyLogger(logger) { | ||
| return logger |
There was a problem hiding this comment.
4. providers.test.jsx violates formatting 📘 Rule violation ✓ Correctness
The modified test file introduces/continues double quotes and missing semicolons, conflicting with the repo’s Prettier standards. Additionally, the file extension is not *.test.ts(x) as required by the test naming rule.
Agent Prompt
## Issue description
`providers.test.jsx` conflicts with repo conventions: formatting diverges from Prettier rules, and the test file extension does not match the required `*.test.ts`/`*.test.tsx` pattern.
## Issue Context
This PR modifies the test file while transitioning to Vitest/MSW patterns; the file should be kept consistent with the repo’s formatting and naming conventions.
## Fix Focus Areas
- packages/next-auth/src/client/__tests__/providers.test.jsx[1-29]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1
to
+22
| 'use strict' | ||
|
|
||
| const fs = require('fs') | ||
| const path = require('path') | ||
|
|
||
| function wildcardToRegExp(pattern) { | ||
| const escapedPattern = pattern.replace(/[|\\{}()[\]^$+?.]/g, '\\$&') | ||
| return new RegExp(`^${escapedPattern.replace(/\*/g, '.*')}$`) | ||
| } | ||
|
|
||
| function cleanMatchingFiles(dir, patterns) { | ||
| if (!fs.existsSync(dir)) return | ||
|
|
||
| const matchers = patterns.map(wildcardToRegExp) | ||
|
|
||
| for (const entry of fs.readdirSync(dir)) { | ||
| if (!matchers.some((matcher) => matcher.test(entry))) continue | ||
| fs.rmSync(path.join(dir, entry), { force: true, recursive: true }) | ||
| } | ||
| } | ||
|
|
||
| module.exports = cleanMatchingFiles |
There was a problem hiding this comment.
5. cleanup-assets.js missing semicolons 📘 Rule violation ✓ Correctness
The new cleanup-assets.js file omits semicolons, conflicting with the repo’s Prettier formatting standards. This introduces inconsistent style and may cause formatting churn in future diffs.
Agent Prompt
## Issue description
`packages/next-pwa/cleanup-assets.js` does not match the repo Prettier rules (notably missing semicolons).
## Issue Context
This is newly added code and should be formatted consistently to prevent style drift.
## Fix Focus Areas
- packages/next-pwa/cleanup-assets.js[1-22]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| console.warn( | ||
| '> [PWA] Warning: workbox-webpack-plugin not installed. Run: npm install workbox-webpack-plugin' | ||
| ); | ||
| return nextConfig; | ||
| } | ||
|
|
||
| try { | ||
| cleanMatchingFiles = require('./cleanup-assets'); |
There was a problem hiding this comment.
6. Missing cleanup-assets in package 🐞 Bug ⛯ Reliability
@opensourceframework/next-pwa now requires "./cleanup-assets" from runtime code, but cleanup-assets.js is neither copied into dist nor included in the npm "files" allowlist, causing MODULE_NOT_FOUND for consumers. This breaks builds at the first execution of withPWA() where helper modules are loaded or when custom/fallback workers run.
Agent Prompt
### Issue description
`@opensourceframework/next-pwa` now requires `./cleanup-assets`, but the build/publish pipeline does not include `cleanup-assets.js` in `dist/` or the npm `files` allowlist, causing `MODULE_NOT_FOUND` for consumers.
### Issue Context
`tsup` builds `dist/index.js` and copies a small set of runtime helper files into `dist/`. Those copied helpers (`dist/build-custom-worker.js`, `dist/build-fallback-worker.js`) also `require('./cleanup-assets')`, which will resolve to `dist/cleanup-assets.js` at runtime.
### Fix Focus Areas
- packages/next-pwa/tsup.config.ts[42-58]
- packages/next-pwa/package.json[36-46]
- packages/next-pwa/index.js[198-205]
- packages/next-pwa/build-custom-worker.js[1-8]
- packages/next-pwa/build-fallback-worker.js[1-8]
### What to change
1. Add `cleanup-assets.js` to the `filesToCopy` list in `tsup.config.ts` so it is copied to `dist/cleanup-assets.js`.
2. Add `cleanup-assets.js` to `package.json` `files` so it’s published.
3. (Optional hardening) Consider adding a small unit test that asserts `require.resolve('@opensourceframework/next-pwa/dist/cleanup-assets.js')` exists after build, or that `withPWA()` can be required/executed without throwing.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+225
to
+238
| async getOAuthRequestToken(params = {}) { | ||
| // Implement OAuth 1.0a request token logic | ||
| throw new Error("OAuth 1.0a is not yet fully implemented in the native client. Please use OAuth 2.0 or contact maintainers.") | ||
| } | ||
|
|
||
| async getOAuthAccessToken(oauth_token, oauth_token_secret, oauth_verifier) { | ||
| // Implement OAuth 1.0a access token logic | ||
| throw new Error("OAuth 1.0a is not yet fully implemented in the native client.") | ||
| } | ||
|
|
||
| async get(url, oauth_token, oauth_token_secret) { | ||
| // Implement OAuth 1.0a authenticated request | ||
| throw new Error("OAuth 1.0a is not yet fully implemented in the native client.") | ||
| } |
There was a problem hiding this comment.
7. Oauth1 client unimplemented 🐞 Bug ✓ Correctness
next-auth now returns an OAuth1Client for non-2.x providers, but OAuth1Client methods throw "not yet fully implemented", so OAuth 1.0a providers cannot sign in. This breaks the OAuth v1 callback/signin flow that calls getOAuthRequestToken/getOAuthAccessToken/get (e.g., Twitter).
Agent Prompt
### Issue description
OAuth 1.x is currently broken: `OAuth1Client` methods throw `"not yet fully implemented"`, but the sign-in and callback flows call these methods for any provider whose `version` does not start with `"2."`.
### Issue Context
At least one shipped provider (`twitter`) is `version: "1.0A"` and relies on `getOAuthRequestToken`, `getOAuthAccessToken`, and `get` to complete authentication.
### Fix Focus Areas
- packages/next-auth/src/server/lib/oauth/client.js[11-21]
- packages/next-auth/src/server/lib/oauth/client.js[210-239]
- packages/next-auth/src/server/lib/signin/oauth.js[14-52]
- packages/next-auth/src/server/lib/oauth/callback.js[71-88]
- packages/next-auth/src/providers/twitter.js[1-12]
### What to change
1. Replace the throwing `OAuth1Client` stub with a real OAuth 1.0a implementation (signature base string, HMAC-SHA1, nonce/timestamp, Authorization header, request/access token exchanges).
2. Add/extend tests that exercise an OAuth1 provider flow (at minimum, verify that the OAuth1 code paths do not throw and that request signing is performed).
3. If OAuth1 support is intentionally being removed, remove/disable OAuth1 providers (like Twitter) and ensure the public API/docs reflect the breaking change (but this PR currently still ships the provider).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Owner
Author
|
|
3 similar comments
Owner
Author
|
|
Owner
Author
|
|
Owner
Author
|
|
- Added 'cleanup-assets.js' to tsup.config.ts onSuccess copy list for dist availability. - Added 'cleanup-assets.js' to package.json files array for npm publication. - This resolves runtime errors when build helpers attempt to require the module.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Owner
Author
|
|
Owner
Author
|
Superseded by #59 (squash merge of all modernization PRs). Changes included in main. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #22 (roadmap).
nextto ^16.0.0 indevDependencies.peerDependenciesto support React 19.