fix: address items from #44. (#45)

Author: knightedcodemonkey

README.md

@@ -125,6 +125,7 @@ type ModuleOptions = {
     | '.mts'
     | '.cts'
     | ((value: string) => string | null | undefined)
+  rewriteTemplateLiterals?: 'allow' | 'static-only'
   dirFilename?: 'inject' | 'preserve' | 'error'
   importMeta?: 'preserve' | 'shim' | 'error'
   importMetaMain?: 'shim' | 'warn' | 'error'
@@ -151,6 +152,7 @@ type ModuleOptions = {
 - `appendJsExtension` (`relative-only` when targeting ESM): append `.js` to relative specifiers; never touches bare specifiers.
 - `appendDirectoryIndex` (`index.js`): when a relative specifier ends with a slash, append this index filename (set `false` to disable).
 - `appenders` precedence: `rewriteSpecifier` runs first; if it returns a string, that result is used. If it returns `undefined` or `null`, `appendJsExtension` and `appendDirectoryIndex` still run. Bare specifiers are never modified by appenders.
+- `rewriteTemplateLiterals` (`allow`): when `static-only`, interpolated template literals are left untouched by specifier rewriting; string literals and non-interpolated templates still rewrite.
 - `dirFilename` (`inject`): inject `__dirname`/`__filename`, preserve existing, or throw.
 - `importMeta` (`shim`): rewrite `import.meta.*` to CommonJS equivalents.
 - `importMetaMain` (`shim`): gate `import.meta.main` with shimming/warning/error when Node support is too old.
@@ -159,7 +161,7 @@ type ModuleOptions = {
 - `detectCircularRequires` (`off`): optionally detect relative static require cycles and warn/throw.
 - `detectDualPackageHazard` (`warn`): flag when `import` and `require` mix for the same package or root/subpath are combined in ways that can resolve to separate module instances (dual packages). Set to `error` to fail the transform.
 - `dualPackageHazardScope` (`file`): `file` preserves the legacy per-file detector; `project` aggregates package usage across all CLI inputs (useful in monorepos/hoisted installs) and emits one diagnostic per package.
-- `topLevelAwait` (`error`): throw, wrap, or preserve when TLA appears in CommonJS output.
+- `topLevelAwait` (`error`): throw, wrap, or preserve when TLA appears in CommonJS output. `wrap` runs the file body inside an async IIFE (exports may resolve after the initial tick); `preserve` leaves `await` at top level, which Node will reject for CJS.
 - `rewriteSpecifier` (off): rewrite relative specifiers to a chosen extension or via a callback. Precedence: the callback (if provided) runs first; if it returns a string, that wins. If it returns `undefined` or `null`, the appenders still apply.
 - `requireSource` (`builtin`): whether `require` comes from Node or `createRequire`.
 - `cjsDefault` (`auto`): bundler-style default interop vs direct `module.exports`.

docs/roadmap.md

@@ -21,3 +21,8 @@ Next:
 
 - Emit source maps and clearer diagnostics for transform choices.
 - Benchmark scope analysis choices: compare `periscopic`, `scope-analyzer`, and `eslint-scope` on fixtures and pick the final adapter.
+
+## Potential Breaking Changes (flag/document clearly)
+
+- Template literal specifier rewriting: if we ever default to skipping interpolated `TemplateLiteral` specifiers, it would change outputs. Current implementation is opt-in via `rewriteTemplateLiterals: 'static-only'` (non-breaking); future default flips would need a major/minor note.
+- Cycle detection hardening: expanding extensions (.ts/.tsx/.mts/.cts) and normalize/realpath paths may surface new cycle warnings/errors, especially on Windows or mixed TS/JS projects.

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@knighted/module",
-  "version": "1.4.0-rc.2",
+  "version": "1.4.0-rc.3",
   "description": "Bidirectional transform for ES modules and CommonJS.",
   "type": "module",
   "main": "dist/module.js",

package.json

@@ -7,22 +7,25 @@ import {
 import { parseArgs } from 'node:util'
 import { readFile, mkdir } from 'node:fs/promises'
 import { dirname, resolve, relative, join } from 'node:path'
-import { builtinModules } from 'node:module'
 import { glob } from 'glob'
 
+import type { TemplateLiteral } from '@oxc-project/types'
+
 import { transform, collectProjectDualPackageHazards } from './module.js'
 import { parse } from './parse.js'
 import { format } from './format.js'
 import { specifier } from './specifier.js'
 import { getLangFromExt } from './utils/lang.js'
 import type { ModuleOptions, Diagnostic } from './types.js'
+import { builtinSpecifiers } from './utils/builtinSpecifiers.js'
 
 const defaultOptions: ModuleOptions = {
   target: 'commonjs',
   sourceType: 'auto',
   transformSyntax: true,
   liveBindings: 'strict',
   rewriteSpecifier: undefined,
+  rewriteTemplateLiterals: 'allow',
   appendJsExtension: undefined,
   appendDirectoryIndex: 'index.js',
   dirFilename: 'inject',
@@ -108,16 +111,6 @@ const colorize = (enabled: boolean) => {
   }
 }
 
-const builtinSpecifiers = new Set<string>(
-  builtinModules
-    .map(mod => (mod.startsWith('node:') ? mod.slice(5) : mod))
-    .flatMap(mod => {
-      const parts = mod.split('/')
-      const base = parts[0]
-      return parts.length > 1 ? [mod, base] : [mod]
-    }),
-)
-
 const collapseSpecifier = (value: string) => value.replace(/['"`+)\s]|new String\(/g, '')
 
 const appendExtensionIfNeeded = (
@@ -195,6 +188,12 @@ const optionsTable = [
     type: 'string',
     desc: 'Rewrite import specifiers (.js/.mjs/.cjs/.ts/.mts/.cts)',
   },
+  {
+    long: 'rewrite-template-literals',
+    short: undefined,
+    type: 'string',
+    desc: 'Rewrite template literals (allow|static-only)',
+  },
   {
     long: 'append-js-extension',
     short: 'j',
@@ -377,6 +376,11 @@ const toModuleOptions = (values: ParsedValues): ModuleOptions => {
   const transformSyntax = parseTransformSyntax(
     values['transform-syntax'] as string | undefined,
   )
+  const rewriteTemplateLiterals =
+    parseEnum(
+      values['rewrite-template-literals'] as string | undefined,
+      ['allow', 'static-only'] as const,
+    ) ?? defaultOptions.rewriteTemplateLiterals
   const appendJsExtension = parseEnum(
     values['append-js-extension'] as string | undefined,
     ['off', 'relative-only', 'all'] as const,
@@ -391,6 +395,7 @@ const toModuleOptions = (values: ParsedValues): ModuleOptions => {
     transformSyntax,
     rewriteSpecifier:
       (values['rewrite-specifier'] as ModuleOptions['rewriteSpecifier']) ?? undefined,
+    rewriteTemplateLiterals,
     appendJsExtension: appendJsExtension,
     appendDirectoryIndex,
     detectCircularRequires:
@@ -513,6 +518,13 @@ const applySpecifierUpdates = async (
 
   const lang = getLangFromExt(filename)
   const updated = await specifier.updateSrc(source, lang, spec => {
+    if (
+      spec.type === 'TemplateLiteral' &&
+      opts.rewriteTemplateLiterals === 'static-only'
+    ) {
+      const node = spec.node as TemplateLiteral
+      if (node.expressions.length > 0) return
+    }
     const normalized = normalizeBuiltinSpecifier(spec.value)
     const rewritten = rewriteSpecifierValue(
       normalized ?? spec.value,

src/cli.ts

@@ -1,4 +1,3 @@
-import { builtinModules } from 'node:module'
 import { dirname, join, resolve as pathResolve } from 'node:path'
 import { readFile as fsReadFile, stat as fsStat } from 'node:fs/promises'
 import type { Node, ParseResult } from 'oxc-parser'
@@ -31,6 +30,7 @@ import type { Diagnostic, ExportsMeta, FormatterOptions } from './types.js'
 import { collectCjsExports } from './utils/exports.js'
 import { collectModuleIdentifiers } from './utils/identifiers.js'
 import { isValidUrl } from './utils/url.js'
+import { builtinSpecifiers } from './utils/builtinSpecifiers.js'
 import { ancestorWalk } from './walk.js'
 
 const isRequireMainMember = (node: Node, shadowed: Set<string>) =>
@@ -41,16 +41,6 @@ const isRequireMainMember = (node: Node, shadowed: Set<string>) =>
   node.property.type === 'Identifier' &&
   node.property.name === 'main'
 
-const builtinSpecifiers = new Set<string>(
-  builtinModules
-    .map(mod => (mod.startsWith('node:') ? mod.slice(5) : mod))
-    .flatMap(mod => {
-      const parts = mod.split('/')
-      const base = parts[0]
-      return parts.length > 1 ? [mod, base] : [mod]
-    }),
-)
-
 const stripQuery = (value: string) =>
   value.includes('?') || value.includes('#') ? (value.split(/[?#]/)[0] ?? value) : value
 

src/format.ts

@@ -14,28 +14,18 @@ import {
 } from './format.js'
 import { getLangFromExt } from './utils/lang.js'
 import type { ModuleOptions, Diagnostic } from './types.js'
-import { builtinModules } from 'node:module'
 import { resolve as pathResolve, dirname as pathDirname, extname, join } from 'node:path'
-import { readFile as fsReadFile, stat } from 'node:fs/promises'
+import { readFile as fsReadFile, stat, realpath } from 'node:fs/promises'
 import { parse as parseModule } from './parse.js'
 import { walk } from './walk.js'
 import { collectModuleIdentifiers } from './utils/identifiers.js'
+import { builtinSpecifiers } from './utils/builtinSpecifiers.js'
 
 type AppendJsExtensionMode = NonNullable<ModuleOptions['appendJsExtension']>
 type DetectCircularRequires = NonNullable<ModuleOptions['detectCircularRequires']>
 
 const collapseSpecifier = (value: string) => value.replace(/['"`+)\s]|new String\(/g, '')
 
-const builtinSpecifiers = new Set<string>(
-  builtinModules
-    .map(mod => (mod.startsWith('node:') ? mod.slice(5) : mod))
-    .flatMap(mod => {
-      const parts = mod.split('/')
-      const base = parts[0]
-      return parts.length > 1 ? [mod, base] : [mod]
-    }),
-)
-
 const appendExtensionIfNeeded = (
   spec: Spec,
   mode: AppendJsExtensionMode,
@@ -118,6 +108,8 @@ const fileExists = async (candidate: string) => {
   }
 }
 
+const normalizePath = async (p: string) => pathResolve(await realpath(p).catch(() => p))
+
 const resolveRequirePath = async (fromFile: string, spec: string, dirIndex: string) => {
   if (!spec.startsWith('./') && !spec.startsWith('../')) return null
   const base = pathResolve(pathDirname(fromFile), spec)
@@ -127,12 +119,19 @@ const resolveRequirePath = async (fromFile: string, spec: string, dirIndex: stri
   if (ext) {
     candidates.push(base)
   } else {
-    candidates.push(`${base}.js`, `${base}.cjs`, `${base}.mjs`)
+    candidates.push(
+      `${base}.js`,
+      `${base}.cjs`,
+      `${base}.mjs`,
+      `${base}.ts`,
+      `${base}.mts`,
+      `${base}.cts`,
+    )
     candidates.push(join(base, dirIndex))
   }
 
   for (const candidate of candidates) {
-    if (await fileExists(candidate)) return candidate
+    if (await fileExists(candidate)) return await normalizePath(candidate)
   }
 
   return null
@@ -180,8 +179,10 @@ const detectCircularRequireGraph = async (
   const visited = new Set<string>()
 
   const dfs = async (file: string, stack: string[]) => {
-    if (visiting.has(file)) {
-      const cycle = [...stack, file]
+    const normalized = await normalizePath(file)
+
+    if (visiting.has(normalized)) {
+      const cycle = [...stack, normalized]
       const msg = `Circular require detected: ${cycle.join(' -> ')}`
       if (mode === 'error') {
         throw new Error(msg)
@@ -191,26 +192,26 @@ const detectCircularRequireGraph = async (
       return
     }
 
-    if (visited.has(file)) return
-    visiting.add(file)
-    stack.push(file)
+    if (visited.has(normalized)) return
+    visiting.add(normalized)
+    stack.push(normalized)
 
-    let deps = cache.get(file)
+    let deps = cache.get(normalized)
     if (!deps) {
-      deps = await collectStaticRequires(file, dirIndex)
-      cache.set(file, deps)
+      deps = await collectStaticRequires(normalized, dirIndex)
+      cache.set(normalized, deps)
     }
 
     for (const dep of deps) {
       await dfs(dep, stack)
     }
 
     stack.pop()
-    visiting.delete(file)
-    visited.add(file)
+    visiting.delete(normalized)
+    visited.add(normalized)
   }
 
-  await dfs(entryFile, [])
+  await dfs(await normalizePath(entryFile), [])
 }
 
 const mergeUsageMaps = (
@@ -271,12 +272,13 @@ const collectProjectDualPackageHazards = async (files: string[], opts: ModuleOpt
   return byFile
 }
 
-const defaultOptions = {
+const createDefaultOptions = (): ModuleOptions => ({
   target: 'commonjs',
   sourceType: 'auto',
   transformSyntax: true,
   liveBindings: 'strict',
   rewriteSpecifier: undefined,
+  rewriteTemplateLiterals: 'allow',
   appendJsExtension: undefined,
   appendDirectoryIndex: 'index.js',
   dirFilename: 'inject',
@@ -295,9 +297,12 @@ const defaultOptions = {
   cwd: undefined,
   out: undefined,
   inPlace: false,
-} satisfies ModuleOptions
-const transform = async (filename: string, options: ModuleOptions = defaultOptions) => {
-  const opts = { ...defaultOptions, ...options, filePath: filename }
+})
+const transform = async (filename: string, options?: ModuleOptions) => {
+  const base = createDefaultOptions()
+  const opts = options
+    ? { ...base, ...options, filePath: filename }
+    : { ...base, filePath: filename }
   const cwdBase = opts.cwd ? resolve(opts.cwd) : process.cwd()
   const appendMode: AppendJsExtensionMode =
     options?.appendJsExtension ?? (opts.target === 'module' ? 'relative-only' : 'off')
@@ -311,6 +316,13 @@ const transform = async (filename: string, options: ModuleOptions = defaultOptio
 
   if (opts.rewriteSpecifier || appendMode !== 'off' || dirIndex) {
     const code = await specifier.updateSrc(source, getLangFromExt(filename), spec => {
+      if (
+        spec.type === 'TemplateLiteral' &&
+        opts.rewriteTemplateLiterals === 'static-only'
+      ) {
+        const node = spec.node as TemplateLiteral
+        if (node.expressions.length > 0) return
+      }
       const normalized = normalizeBuiltinSpecifier(spec.value)
       const rewritten = rewriteSpecifierValue(
         normalized ?? spec.value,

src/module.ts

@@ -34,6 +34,8 @@ export type ModuleOptions = {
   liveBindings?: 'strict' | 'loose' | 'off'
   /** Rewrite import specifiers (e.g. add extensions). */
   rewriteSpecifier?: RewriteSpecifier
+  /** Whether to rewrite template literals that contain expressions. Default allows rewrites; set to 'static-only' to skip interpolated templates. */
+  rewriteTemplateLiterals?: 'allow' | 'static-only'
   /** Whether to append .js to relative imports. */
   appendJsExtension?: 'off' | 'relative-only' | 'all'
   /** Add directory index (e.g. /index.js) or disable. */

src/types.ts

@@ -0,0 +1,13 @@
+import { builtinModules } from 'node:module'
+
+const builtinSpecifiers = new Set<string>(
+  builtinModules
+    .map(mod => (mod.startsWith('node:') ? mod.slice(5) : mod))
+    .flatMap(mod => {
+      const parts = mod.split('/')
+      const base = parts[0]
+      return parts.length > 1 ? [mod, base] : [mod]
+    }),
+)
+
+export { builtinSpecifiers }

src/utils/builtinSpecifiers.ts

@@ -638,6 +638,33 @@ test('rewrites specifiers with --rewrite-specifier', () => {
   assert.match(result.stdout, /\.\/foo\.js'/)
 })
 
+test('--rewrite-template-literals guards interpolated templates', () => {
+  const source = [
+    "const side = 'alpha'",
+    "import './file.ts'",
+    'import(`./tmpl/${side}.ts`)',
+    '',
+  ].join('\n')
+
+  const result = runCli(
+    [
+      '--target',
+      'module',
+      '--stdin-filename',
+      'input.mjs',
+      '--rewrite-specifier',
+      '.js',
+      '--rewrite-template-literals',
+      'static-only',
+    ],
+    source,
+  )
+
+  assert.equal(result.status, 0)
+  assert.ok(result.stdout.includes("import './file.js'"))
+  assert.ok(result.stdout.includes('import(`./tmpl/${side}.ts`)'))
+})
+
 test('help example: out-dir mirror', async t => {
   const temp = await mkdtemp(join(tmpdir(), 'module-cli-'))
   const srcDir = join(temp, 'src')