Skip to content

feat: add validation for root-level files when src directory exists #82514

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

Open
wants to merge 3 commits into
base: canary
Choose a base branch
from

Conversation

devin-ai-integration[bot]
Copy link
Contributor

What?

This PR adds validation to throw an error when root-level files (middleware and instrumentation) are found outside the src/ directory when a src/ directory structure is being used.

Why?

Currently, Next.js allows middleware and instrumentation files to exist at the project root even when using a src/ directory structure. This can lead to confusion and inconsistent file organization. The Next.js documentation states that when using the src directory, middleware should be placed inside the src folder, but this wasn't enforced.

How?

  1. Added new error class: SrcDirectoryError in packages/next/src/build/utils.ts that provides clear error messaging and documentation links

  2. Added validation functions:

    • validateMiddlewareInSrcDir(fileName: string, isSrcDir: boolean)
    • validateInstrumentationInSrcDir(fileName: string, isSrcDir: boolean)
  3. Integrated validation in two key locations:

    • Development: packages/next/src/server/lib/router-utils/setup-dev-bundler.ts - validates during dev server startup
    • Build time: packages/next/src/build/index.ts - validates during production build
  4. Added test cases: Created test/integration/src-dir-validation/ to verify the validation behavior

The validation logic checks:

  • If src/ directory structure is being used (isSrcDir)
  • If the file is a middleware or instrumentation file
  • If the file is located outside the src/ directory
  • If all conditions are true, throws SrcDirectoryError with helpful guidance

⚠️ Human Review Checklist

Critical items to verify:

  1. File path logic: Verify that fileName.startsWith('/src/') correctly identifies files inside src directory for Next.js internal file paths
  2. Test execution: Run the new tests in test/integration/src-dir-validation/ to ensure they work correctly
  3. Existing tests: Confirm existing middleware-related tests still pass (especially test/integration/middleware-src)
  4. Dev vs Build consistency: Test that validation works identically in both next dev and next build scenarios
  5. Error message accuracy: Verify the documentation link in the error message is correct and helpful
  6. Edge cases: Test scenarios like:
    • Projects without src directory (should not validate)
    • Projects with src directory but no middleware/instrumentation (should not error)
    • Mixed scenarios with some files in src and some outside

⚠️ Testing Gap: This implementation could not be tested locally due to missing pnpm dependency. The validation logic and integration points need thorough testing to ensure correctness.

Breaking Change Consideration: This introduces new fatal errors for previously "working" (but incorrectly organized) projects. Consider if this should be a warning first before becoming an error in a future version.


Link to Devin run: https://app.devin.ai/sessions/94e9f081963a4ec3bc8bc9c8c1ca8975
Requested by: @devjiwonchoi

- Add SrcDirectoryError class for consistent error messaging
- Add validateMiddlewareInSrcDir and validateInstrumentationInSrcDir functions
- Integrate validation in both dev bundler and build processes
- Throw error when middleware/instrumentation files are outside src/ when src/ directory exists
- Add test cases to verify validation behavior

Co-Authored-By: Jiwon Choi <devjiwonchoi@gmail.com>
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@ijjk
Copy link
Member

ijjk commented Aug 10, 2025

Allow CI Workflow Run

  • approve CI run for commit: ef4bcb1

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@ijjk
Copy link
Member

ijjk commented Aug 10, 2025

Allow CI Workflow Run

  • approve CI run for commit: 8bafaef

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

}

export function validateMiddlewareInSrcDir(fileName: string, isSrcDir: boolean) {
if (isSrcDir && isMiddlewareFile(fileName) && !fileName.startsWith('/src/')) {
Copy link

@vercel vercel bot Aug 10, 2025

Choose a reason for hiding this comment

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

The new validation functions use hardcoded /src/ path prefixes which will fail on Windows systems where paths use backslashes, causing false positive errors even when files are correctly placed in the src directory.

View Details

Analysis

The validateMiddlewareInSrcDir and validateInstrumentationInSrcDir functions check if file paths start with /src/ using fileName.startsWith('/src/'). However, in the build process, the rootPaths are created using path.join(rootDir, file).replace(dir, '') which preserves platform-specific path separators.

On Windows systems, this creates paths like \src\middleware.js (with backslashes), but the validation checks for /src/ (with forward slashes). This means:

  1. On Windows: A middleware file correctly placed in src/middleware.js will have a path like \src\middleware.js
  2. The validation: !fileName.startsWith('/src/') will return true (since \src\middleware.js doesn't start with /src/)
  3. Result: The validation throws SrcDirectoryError even though the file is in the correct location

This is inconsistent with the dev server code path, which uses normalizePathSep() to convert backslashes to forward slashes before validation, ensuring cross-platform compatibility.

The codebase has established patterns for handling cross-platform paths (see normalizePathSep(), ensureLeadingSlash() functions), but the new validation functions don't follow these patterns.


Recommendation

Normalize the path separators before validation to ensure cross-platform compatibility. Update both validation functions to use forward slashes consistently:

export function validateMiddlewareInSrcDir(fileName: string, isSrcDir: boolean) {
  const normalizedPath = fileName.replace(/\\/g, '/')
  if (isSrcDir && isMiddlewareFile(fileName) && !normalizedPath.startsWith('/src/')) {
    throw new SrcDirectoryError(fileName, 'middleware')
  }
}

export function validateInstrumentationInSrcDir(fileName: string, isSrcDir: boolean) {
  const normalizedPath = fileName.replace(/\\/g, '/')
  if (isSrcDir && isInstrumentationHookFile(fileName) && !normalizedPath.startsWith('/src/')) {
    throw new SrcDirectoryError(fileName, 'instrumentation')
  }
}

Alternatively, import and use the existing normalizePathSep function from shared/lib/page-path/normalize-path-sep.ts for consistency with the rest of the codebase.

Co-Authored-By: Jiwon Choi <devjiwonchoi@gmail.com>
Comment on lines 1167 to 1172
if (rootPath.includes(MIDDLEWARE_FILENAME)) {
validateMiddlewareInSrcDir(rootPath, isSrcDir)
}
if (rootPath.includes(INSTRUMENTATION_HOOK_FILENAME)) {
validateInstrumentationInSrcDir(rootPath, isSrcDir)
}
Copy link

@vercel vercel bot Aug 10, 2025

Choose a reason for hiding this comment

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

The src directory validation for middleware and instrumentation files never triggers during builds because the validation functions expect paths without file extensions, but receive paths with extensions.

View Details

Analysis

The build process creates rootPaths that contain file extensions (like /middleware.js, /src/middleware.js) through this processing:

const rootPaths = Array.from(await getFilesInDir(rootDir))
  .filter((file) => includes.some((include) => include.test(file)))
  .map((file) => path.join(rootDir, file).replace(dir, ''))

However, the validation functions validateMiddlewareInSrcDir() and validateInstrumentationInSrcDir() rely on isMiddlewareFile() and isInstrumentationHookFile() which only match exact paths without extensions:

export function isMiddlewareFile(file: string) {
  return (
    file === `/${MIDDLEWARE_FILENAME}` || file === `/src/${MIDDLEWARE_FILENAME}`
  )
}

This means isMiddlewareFile('/middleware.js') returns false, causing the validation condition to never trigger. The validation works correctly in development mode because absolutePathToPage() strips file extensions, but fails during builds.

This bug means the new validation feature is completely non-functional during builds, potentially allowing users to incorrectly place middleware/instrumentation files outside the src directory when they should be required to be inside it.


Recommendation

Strip the file extension from the path before calling the validation functions. Modify the validation calls in lines 1167-1172:

if (rootPath.includes(MIDDLEWARE_FILENAME)) {
  const pathWithoutExtension = rootPath.replace(/\.[^/.]+$/, '')
  validateMiddlewareInSrcDir(pathWithoutExtension, isSrcDir)
}
if (rootPath.includes(INSTRUMENTATION_HOOK_FILENAME)) {
  const pathWithoutExtension = rootPath.replace(/\.[^/.]+$/, '')
  validateInstrumentationInSrcDir(pathWithoutExtension, isSrcDir)
}

Alternatively, update the validation functions to handle paths with extensions by modifying isMiddlewareFile and isInstrumentationHookFile to strip extensions before comparison.

…ion issues

- Use normalizePathSep for cross-platform path handling
- Strip file extensions before validation in build context
- Ensure validation works consistently in dev and build modes

Co-Authored-By: Jiwon Choi <devjiwonchoi@gmail.com>
@@ -1157,6 +1161,19 @@ export default async function build(

NextBuildContext.hasInstrumentationHook = hasInstrumentationHook

if (isSrcDir) {
Copy link

Choose a reason for hiding this comment

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

The build process fails to detect middleware and instrumentation files placed at the project root when using src directory structure, preventing helpful validation errors from being shown to users.

View Details

Analysis

When using a src directory structure (src/pages or src/app), the build process should detect if middleware or instrumentation files are incorrectly placed at the project root instead of inside the src/ directory, and throw a clear error to guide users.

However, there's a bug in the file detection logic on line 1144. The code calculates:

const rootDir = path.join((pagesDir || appDir)!, '..')

When pagesDir is /project/src/pages, this results in rootDir = /project/src. The subsequent getFilesInDir(rootDir) call only searches within the src directory, completely missing files that are incorrectly placed at the actual project root (/project/).

This means users who place middleware.js or instrumentation.js at the project root while using src directory structure won't receive the helpful build-time validation error. Instead, they'll experience confusing runtime behavior without clear guidance on where to place their files.

The test case "should throw error when middleware is outside src directory" expects this validation to work, confirming this is a bug rather than intended behavior.


Recommendation

Change the rootDir calculation to properly search the project root directory. Replace line 1144 with:

const projectRoot = path.join((pagesDir || appDir)!, '../..')
const srcRoot = path.join((pagesDir || appDir)!, '..')
const rootDir = isSrcDir ? projectRoot : srcRoot

Or alternatively, search both locations when isSrcDir is true:

const searchDirs = isSrcDir ? 
  [path.join((pagesDir || appDir)!, '../..'), path.join((pagesDir || appDir)!, '..')] : 
  [path.join((pagesDir || appDir)!, '..')]

This ensures that when using src directory structure, the validation checks both the project root (to catch incorrectly placed files) and the src directory (to allow correctly placed files).

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.

1 participant