Implement dual publish types format check

pkg/index.d.ts

@@ -63,7 +63,24 @@ export type Message =
   | BaseMessage<'EXPORTS_MODULE_SHOULD_BE_ESM'>
   | BaseMessage<'EXPORTS_VALUE_INVALID', { suggestValue: string }>
   | BaseMessage<'USE_EXPORTS_BROWSER'>
-  | BaseMessage<'TYPES_NOT_EXPORTED', { typesFilePath: string }>
+  | BaseMessage<
+      'TYPES_NOT_EXPORTED',
+      {
+        typesFilePath: string
+        actualExtension?: string
+        expectExtension?: string
+      }
+    >
+  | BaseMessage<
+      'EXPORT_TYPES_INVALID_FORMAT',
+      {
+        condition: string
+        actualFormat: string
+        expectFormat: string
+        actualExtension: string
+        expectExtension: string
+      }
+    >
 
 export interface Options {
   /**

pkg/src/index.js

@@ -11,7 +11,9 @@ import {
   isFileContentLintable,
   getAdjacentDtsPath,
   resolveExports,
-  isDtsFile
+  isDtsFile,
+  getDtsFilePathFormat,
+  getDtsCodeFormatExtension
 } from './utils.js'
 
 /**
@@ -554,7 +556,7 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) {
         checkTypesExported()
       }
       // else this `exports` may have multiple export entrypoints, check for '.'
-      // TODO: check for other entrypoints
+      // TODO: check for other entrypoints, move logic into `crawlExports`
       else if ('.' in exports) {
         checkTypesExported('.')
       }
@@ -577,7 +579,8 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) {
           ? exportsPkgPath.concat(exportsRootKey)
           : exportsPkgPath
 
-        const seenPathStrings = new Set()
+        // keyed strings for seen resolved paths, so we don't trigger duplicate messages for the same thing
+        const seenResolvedKeys = new Set()
 
         // NOTE: got lazy. here we check for the import/require result in different environments
         // to make sure we cover possible cases. however, a better way it to resolve the exports
@@ -593,18 +596,74 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) {
 
             if (!result) continue
 
-            const pathString = result.path.join('.')
-            if (seenPathStrings.has(pathString)) continue
-            seenPathStrings.add(pathString)
-
-            if (!isDtsFile(result.value)) {
+            // check if we've seen this resolve before. we also key by format as we want to distinguish
+            // incorrect exports, but only when the "exports -> path" contains that format, otherwise
+            // it's intentional fallback behaviour by libraries and we don't want to trigger a false alarm.
+            // e.g. libraries that only `"exports": "./index.mjs"` means it's ESM only, so we don't key
+            // the format, so the next run with `"require"` condition is skipped.
+            // different env can share the same key as code can usually be used for multiple environments.
+            const seenKey =
+              result.path.join('.') + (result.dualPublish ? format : '')
+            if (seenResolvedKeys.has(seenKey)) continue
+            seenResolvedKeys.add(seenKey)
+
+            if (isDtsFile(result.value)) {
+              // if we have resolve to a dts file, it might not be ours because typescript requires
+              // `.d.mts` and `.d.cts` for esm and cjs (`.js` and nearest type: module behaviour applies).
+              // check if we're hitting this case :(
+              const dtsActualFormat = await getDtsFilePathFormat(
+                vfs.pathJoin(pkgDir, result.value),
+                vfs
+              )
+              // get the intended format from the conditions. yes, while the `import` condition can actually
+              // point to a CJS file, what we're checking here is the intent of the expected format.
+              // otherwise the package could currently have the wrong format (which we would emit a message above),
+              // but when we reach here, we don't want to base on that incorrect format.
+              const dtsExpectFormat = format === 'import' ? 'ESM' : 'CJS'
+              if (dtsActualFormat !== dtsExpectFormat) {
+                messages.push({
+                  code: 'EXPORT_TYPES_INVALID_FORMAT',
+                  args: {
+                    condition: format,
+                    actualFormat: dtsActualFormat,
+                    expectFormat: dtsExpectFormat,
+                    actualExtension: vfs.getExtName(result.value),
+                    expectExtension: getDtsCodeFormatExtension(dtsExpectFormat)
+                  },
+                  path: result.path,
+                  type: 'warning'
+                })
+              }
+            } else {
+              // adjacent dts file here is always in the correct format
               const hasAdjacentDtsFile = await vfs.isPathExist(
                 vfs.pathJoin(pkgDir, getAdjacentDtsPath(result.value))
               )
+              // if there's no adjacent dts file, it's likely they don't support moduleResolution: bundler.
+              // try to provide a warning.
               if (!hasAdjacentDtsFile) {
+                // before we recommend using `typesFilePath` for this export condition, we need to make sure
+                // it's of a matching format
+                const dtsActualFormat = await getDtsFilePathFormat(
+                  vfs.pathJoin(pkgDir, typesFilePath),
+                  vfs
+                )
+                const dtsExpectFormat = format === 'import' ? 'ESM' : 'CJS'
+                // if it's a matching format, we can recommend using the types file for this exports condition too.
+                // if not, we need to tell them to create a `.d.[mc]ts` file and not use `typesFilePath`.
+                // this is signalled in `matchingFormat`, where the message handler should check for it.
+                const isMatchingFormat = dtsActualFormat === dtsExpectFormat
                 messages.push({
                   code: 'TYPES_NOT_EXPORTED',
-                  args: { typesFilePath },
+                  args: {
+                    typesFilePath,
+                    actualExtension: isMatchingFormat
+                      ? undefined
+                      : vfs.getExtName(typesFilePath),
+                    expectExtension: isMatchingFormat
+                      ? undefined
+                      : getDtsCodeFormatExtension(dtsExpectFormat)
+                  },
                   path: result.path,
                   type: 'warning'
                 })

pkg/src/message.js

@@ -73,16 +73,42 @@ export function printMessage(m, pkg) {
       // prettier-ignore
       return `${c.bold('pkg.browser')} can be refactored to use ${c.bold('pkg.exports')} and the ${c.bold('"browser"')} condition instead to declare browser-specific exports. (This will be a breaking change)`
     case 'TYPES_NOT_EXPORTED': {
-      let target = fp(m.path)
-      if (target.endsWith('.exports')) {
-        target = 'The library'
+      const typesFilePath = exportsRel(m.args.typesFilePath)
+      if (m.args.actualExtension && m.args.expectExtension) {
+        // prettier-ignore
+        return `${c.bold(fp(m.path))} types is not exported. Consider adding ${c.bold(fp(m.path) + '.types')} to be compatible with TypeScript's ${c.bold('"moduleResolution": "bundler"')} compiler option. `
+          + `Note that you cannot use "${c.bold(typesFilePath)}" because it has a mismatching format. Instead, you can duplicate the file and use the ${c.bold(m.args.expectExtension)} extension, e.g. `
+          + `${c.bold(fp(m.path) + '.types')}: "${c.bold(typesFilePath.replace(m.args.actualExtension, m.args.expectExtension))}"`
       } else {
-        target = c.bold(target)
+        // prettier-ignore
+        return `${c.bold(fp(m.path))} types is not exported. Consider adding ${c.bold(fp(m.path) + '.types')}: "${c.bold(typesFilePath)}" to be compatible with TypeScript's ${c.bold('"moduleResolution": "bundler"')} compiler option.`
+      }
+    }
+    case 'EXPORT_TYPES_INVALID_FORMAT': {
+      // convert ['exports', 'types'] -> ['exports', '<condition>', 'types']
+      // convert ['exports', 'types', 'node'] -> ['exports', 'types', 'node', '<condition>']
+      const expectPath = m.path.slice()
+      const typesIndex = m.path.findIndex((p) => p === 'types')
+      if (typesIndex === m.path.length - 1) {
+        expectPath.splice(typesIndex, 0, m.args.condition)
+      } else {
+        expectPath.push(m.args.condition)
       }
       // prettier-ignore
-      return `${target} has types at ${c.bold(m.args.typesFilePath)} but it is not exported from ${c.bold('pkg.exports')}. Consider adding it to ${c.bold(fp(m.path) + '.types')} to be compatible with TypeScript's ${c.bold('"moduleResolution": "bundler"')} compiler option.`
+      return `${c.bold(fp(m.path))} types is an invalid format when resolving with the "${c.bold(m.args.condition)}" condition. Consider splitting out two ${c.bold("types")} condition for ${c.bold("import")} and ${c.bold("require")}, and use the ${c.yellow(m.args.expectExtension)} extension, `
+        + `e.g. ${c.bold(fp(expectPath))}: "${c.bold(pv(m.path).replace(m.args.actualExtension, m.args.expectExtension))}"`
     }
     default:
       return
   }
 }
+
+/**
+ * Make sure s is an `"exports"` compatible relative path
+ * @param {string} s
+ */
+function exportsRel(s) {
+  if (s[0] === '.') return s
+  if (s[0] === '/') return '.' + s
+  return './' + s
+}

pkg/src/utils.js

@@ -146,7 +146,7 @@ function slash(str) {
 /**
  * @param {string} filePath
  * @param {import('../index.d.ts').Vfs} vfs
- * @returns {Promise<CodeFormat>}
+ * @returns {Promise<Exclude<CodeFormat, 'mixed' | 'unknown'>>}
  */
 export async function getFilePathFormat(filePath, vfs) {
   // React Native bundler treats `.native.js` as special platform extension, the real format
@@ -161,6 +161,18 @@ export async function getFilePathFormat(filePath, vfs) {
   return nearestPkg?.type === 'module' ? 'ESM' : 'CJS'
 }
 
+/**
+ * @param {string} filePath
+ * @param {import('../index.d.ts').Vfs} vfs
+ * @returns {Promise<Exclude<CodeFormat, 'mixed' | 'unknown'>>}
+ */
+export async function getDtsFilePathFormat(filePath, vfs) {
+  if (filePath.endsWith('.d.mts')) return 'ESM'
+  if (filePath.endsWith('.d.cts')) return 'CJS'
+  const nearestPkg = await getNearestPkg(filePath, vfs)
+  return nearestPkg?.type === 'module' ? 'ESM' : 'CJS'
+}
+
 /**
  * @param {CodeFormat} format
  */
@@ -175,6 +187,20 @@ export function getCodeFormatExtension(format) {
   }
 }
 
+/**
+ * @param {CodeFormat} format
+ */
+export function getDtsCodeFormatExtension(format) {
+  switch (format) {
+    case 'ESM':
+      return '.mts'
+    case 'CJS':
+      return '.cts'
+    default:
+      return '.ts'
+  }
+}
+
 /**
  * @param {string} path
  */
@@ -325,23 +351,40 @@ export function isDtsFile(filePath) {
  * @param {Record<string, any> | string | string[]} exportsValue
  * @param {string[]} conditions
  * @param {string[]} [currentPath] matched conditions while resolving the exports
- * @returns {{ value: string, path: string[] } | undefined}
+ * @param {{ dualPublish: boolean }} [_metadata]
+ * @returns {{ value: string, path: string[], dualPublish: boolean } | undefined}
  */
-export function resolveExports(exportsValue, conditions, currentPath = []) {
+export function resolveExports(
+  exportsValue,
+  conditions,
+  currentPath = [],
+  _metadata = { dualPublish: false }
+) {
   if (typeof exportsValue === 'string') {
-    return { value: exportsValue, path: currentPath }
+    // prettier-ignore
+    return { value: exportsValue, path: currentPath, dualPublish: _metadata.dualPublish }
   } else if (Array.isArray(exportsValue)) {
-    return { value: exportsValue[0], path: currentPath }
+    // prettier-ignore
+    return { value: exportsValue[0], path: currentPath, dualPublish: _metadata.dualPublish }
+  }
+
+  // while traversing the exports object, also keep info it the path we're traversing
+  // intends to dual export. helpful for better logging heuristics.
+  if (
+    _metadata.dualPublish === false &&
+    ('import' in exportsValue || 'require' in exportsValue)
+  ) {
+    _metadata.dualPublish = true
   }
 
   for (const key in exportsValue) {
     if (conditions.includes(key) || key === 'default') {
       return resolveExports(
         exportsValue[key],
         conditions,
-        currentPath.concat(key)
+        currentPath.concat(key),
+        _metadata
       )
     }
   }
 }
-

pkg/tests/fixtures/types-exports-resolution-dual-explicit/package.json

@@ -0,0 +1,13 @@
+{
+  "name": "publint-types-exports-resolution-dual",
+  "version": "0.0.1",
+  "private": true,
+  "types": "./main-special.d.ts",
+  "exports": {
+    ".": {
+      "types": "./main-special.d.ts",
+      "import": "./main.js",
+      "require": "./main.cjs"
+    }
+  }
+}

pkg/tests/playground.js

@@ -23,7 +23,8 @@ testFixture('glob-deprecated', [
 testFixture('missing-files', [
   'USE_EXPORTS_BROWSER',
   ...Array(7).fill('FILE_DOES_NOT_EXIST'),
-  'FILE_NOT_PUBLISHED'
+  'FILE_NOT_PUBLISHED',
+  'EXPORT_TYPES_INVALID_FORMAT'
 ])
 
 testFixture('no-exports-module', [])
@@ -91,6 +92,10 @@ testFixture('types-exports-resolution-dual', [
   'TYPES_NOT_EXPORTED'
 ])
 
+testFixture('types-exports-resolution-dual-explicit', [
+  'EXPORT_TYPES_INVALID_FORMAT'
+])
+
 testFixture('types-versions', [])
 
 /**

site/src/utils/message.js

@@ -76,14 +76,42 @@ function messageToString(m, pkg) {
       // prettier-ignore
       return `${bold('pkg.browser')} can be refactored to use ${bold('pkg.exports')} and the ${bold('"browser"')} condition instead to declare browser-specific exports. (This will be a breaking change)`
     case 'TYPES_NOT_EXPORTED': {
-      let target = 'This entrypoint'
-      if (m.path[m.path.length - 1] === 'exports') {
-        target = 'The library'
+      const typesFilePath = exportsRel(m.args.typesFilePath)
+      if (m.args.actualExtension && m.args.expectExtension) {
+        // prettier-ignore
+        return `The types is not exported. Consider adding ${bold(fp(m.path) + '.types')} to be compatible with TypeScript's ${bold('"moduleResolution": "bundler"')} compiler option. `
+            + `Note that you cannot use "${bold(typesFilePath)}" because it has a mismatching format. Instead, you can duplicate the file and use the ${bold(m.args.expectExtension)} extension, e.g. `
+            + `${bold(fp(m.path) + '.types')}: "${bold(typesFilePath.replace(m.args.actualExtension, m.args.expectExtension))}"`
+      } else {
+        // prettier-ignore
+        return `The types is not exported. Consider adding ${bold(fp(m.path) + '.types')}: "${bold(typesFilePath)}" to be compatible with TypeScript's ${bold('"moduleResolution": "bundler"')} compiler option.`
+      }
+    }
+    case 'EXPORT_TYPES_INVALID_FORMAT': {
+      // convert ['exports', 'types'] -> ['exports', '<condition>', 'types']
+      // convert ['exports', 'types', 'node'] -> ['exports', 'types', 'node', '<condition>']
+      const expectPath = m.path.slice()
+      const typesIndex = m.path.findIndex((p) => p === 'types')
+      if (typesIndex === m.path.length - 1) {
+        expectPath.splice(typesIndex, 0, m.args.condition)
+      } else {
+        expectPath.push(m.args.condition)
       }
       // prettier-ignore
-      return `${target} has types at ${bold(m.args.typesFilePath)} but it is not exported from ${bold('pkg.exports')}. Consider adding it to ${bold(fp(m.path) + '.types')} to be compatible with TypeScript's ${bold('"moduleResolution": "bundler"')} compiler option.`
+      return `The types is an invalid format when resolving with the "${bold(m.args.condition)}" condition. Consider splitting out two ${bold("types")} condition for ${bold("import")} and ${bold("require")}, and use the ${warn(m.args.expectExtension)} extension, `
+          + `e.g. ${bold(fp(expectPath))}: "${bold(pv(m.path).replace(m.args.actualExtension, m.args.expectExtension))}"`
     }
     default:
       return
   }
 }
+
+/**
+ * Make sure s is an `"exports"` compatible relative path
+ * @param {string} s
+ */
+function exportsRel(s) {
+  if (s[0] === '.') return s
+  if (s[0] === '/') return '.' + s
+  return './' + s
+}

site/src/utils/worker.js

@@ -47,7 +47,7 @@ self.addEventListener('message', async (e) => {
   /** @type {import('publint').Vfs} */
   const vfs = {
     getDirName: (path) => path.replace(/\/[^/]*$/, ''),
-    getExtName: (path) => path.replace(/^.*\./, ''),
+    getExtName: (path) => path.replace(/^.*\./, '.'),
     isPathDir: async (path) => {
       path = path.endsWith('/') ? path : path + '/'
       return files.some((file) => file.name.startsWith(path))