fix: resolve directory imports for next/font shims

[hoangnv170752]

1/ Summary

• Fix directory imports for next/font/local and next/font/google when imported from third-party packages like geist
• Add resolveId hook handling for imports without .js extension that Vite's ESM resolution rejects
  2/ Problem
• Libraries like geist import next/font/local as a directory:
  import localFont from "next/font/local"
• This fails in Vite's ESM mode with:

Directory import '/node_modules/next/font/local' is not supported resolving ES modules
Did you mean to import "next/font/local/index.js"?

The existing resolve.alias only works for user code, not for dependencies in node_modules.
3/ Solution

• Extend the resolveId hook to catch directory-style imports (without .js extension) and resolve them through the shim map, similar to the existing handling for .js extension imports.

[southpolesteve]

/bigbonk review

[ask-bonk]

Thanks for the PR! The intent is sound — catching next/font/local and next/font/google directory-style imports from third-party packages like geist is a real need. However, I have a few concerns:

1. Redundancy with resolve.alias

The nextShimMap is already used as resolve.alias (line 1917), which maps "next/font/local" and "next/font/google" to their shim paths. Vite aliases apply globally, including to imports from node_modules. Can you confirm that resolve.alias genuinely fails for the geist package case? The error you quoted ("Directory import ... is not supported resolving ES modules") typically comes from Node's ESM module resolver, not from Vite's alias plugin. If this is reproducible, please document why the alias fails in the comment.

2. Unify with the existing .js handler

The existing .js handler (lines 2094-2105) and this new block do fundamentally the same thing: look up the import in nextShimMap and return the shim path. They should be merged into a single block:

// Handle next/* imports that resolve.alias may not catch:
// - With .js extension (e.g., "next/navigation.js" from nuqs)
// - Without extension from node_modules (e.g., "next/font/local" from geist)
// Strip .js if present, then resolve through our shim map.
if (cleanId.startsWith("next/")) {
  const baseId = cleanId.endsWith(".js") ? cleanId.slice(0, -3) : cleanId;
  if (nextShimMap[baseId]) {
    const shimPath = nextShimMap[baseId];
    return shimPath.endsWith(".js") ? shimPath : shimPath + ".js";
  }
}

This is cleaner — one block handles both cases with the same logic.

3. No tests

This is a resolution-level change. Please add a test that verifies the resolveId handler returns the correct shim path for bare next/font/local and next/font/google imports without .js.

See inline comments for more detail.

[ask-bonk]

The condition cleanId.startsWith("next/") && !cleanId.endsWith(".js") matches every non-.js next/* import — not just "directory imports". For example, "next/navigation", "next/link", and "next/router" all match this condition.

This is benign (the shim map has those entries and the alias would resolve them anyway), but the comment "Handle directory imports without extension" is misleading — it implies only directory-like paths are caught, when in reality all next/* bare imports match.

Consider either:

1. Making the comment accurate: "Fallback for any next/* bare import not resolved by resolve.alias"
2. Or narrowing the condition if you truly only want directory-style subpath imports (e.g., paths with more than one / segment after next/)

[ask-bonk]

This logic is identical to what the .js handler above does (lines 2101-2103). Rather than duplicating, consider merging both blocks into a single handler as suggested in the review summary. This reduces code duplication and makes the intent clearer.

[ask-bonk]

The review has been submitted successfully. Here's a summary of the feedback I posted:

Review: Changes Requested on PR #157

Three main concerns raised:

1. Redundancy with resolve.alias — The nextShimMap is already used as resolve.alias (line 1917), which should already handle "next/font/local" and "next/font/google" imports globally, including from node_modules. Asked the author to confirm that resolve.alias genuinely fails for the geist package case and document why.

2. Unify with the existing .js handler — The new block and the existing .js extension handler (lines 2094-2105) do the same thing. Suggested merging both into a single, cleaner block that strips .js if present, then looks up the base ID in the shim map.

3. No tests — Requested the author add tests verifying the resolveId handler returns the correct shim path for bare next/font/local and next/font/google imports.

Two inline comments were also posted:

• On line 2110: The condition is overly broad — it matches all next/* imports, not just "directory imports" as the comment claims.
• On line 2113: The logic duplicates the existing .js handler; should be unified.

github run