publint - npm Package Compare versions

Comparing version 0.3.13 to 0.3.14

package.json

 {
   "name": "publint",
-  "version": "0.3.13",
+  "version": "0.3.14",
   "description": "Lint packaging errors",
@@ -5,0 +5,0 @@ "type": "module",

src/index.d.ts

@@ -143,2 +143,3 @@ export type MessageType = 'suggestion' | 'warning' | 'error'
   | BaseMessage<'IMPORTS_FALLBACK_ARRAY_USE'>
+  | BaseMessage<'CJS_WITH_ESMODULE_DEFAULT_EXPORT', { filePath?: string }>
 
@@ -145,0 +146,0 @@ export interface PackFile {

src/shared/core.js

@@ -34,2 +34,3 @@ import {
   isFilePathRawTs,
+  hasEsModuleAndExportsDefault,
 } from './utils.js'
@@ -188,2 +189,13 @@
         }
+        if (
+          expectFormat === 'CJS' &&
+          hasEsModuleAndExportsDefault(defaultContent)
+        ) {
+          messages.push({
+            code: 'CJS_WITH_ESMODULE_DEFAULT_EXPORT',
+            args: { filePath: '/index.js' },
+            path: ['name'],
+            type: 'warning',
+          })
+        }
       }
@@ -239,2 +251,17 @@ })
       }
+      // This check only matters if bundlers uses the file.
+      // If module field or exports field is specified, bundlers will prefer those over main field
+      if (
+        module == null &&
+        exports == null &&
+        expectFormat === 'CJS' &&
+        hasEsModuleAndExportsDefault(mainContent)
+      ) {
+        messages.push({
+          code: 'CJS_WITH_ESMODULE_DEFAULT_EXPORT',
+          args: { filePath: '/' + vfs.pathRelative(pkgDir, mainPath) },
+          path: mainPkgPath,
+          type: 'warning',
+        })
+      }
     })
@@ -360,22 +387,3 @@ }
   if (browser) {
-    crawlBrowser(browser, browserPkgPath)
-    // if the package has both the `browser` and `exports` fields, recommend to use
-    // the browser condition instead
-    if (exports) {
-      if (typeof browser === 'string') {
-        messages.push({
-          code: 'USE_EXPORTS_BROWSER',
-          args: {},
-          path: browserPkgPath,
-          type: 'suggestion',
-        })
-      } else {
-        messages.push({
-          code: 'USE_EXPORTS_OR_IMPORTS_BROWSER',
-          args: {},
-          path: browserPkgPath,
-          type: 'suggestion',
-        })
-      }
-    }
+    crawlBrowser(browser, browserPkgPath, !!exports)
   }
@@ -657,14 +665,63 @@
    * @param {string[]} currentPath
+   * @param {boolean} hasExports
    */
-  function crawlBrowser(fieldValue, currentPath) {
+  function crawlBrowser(fieldValue, currentPath, hasExports) {
     if (typeof fieldValue === 'string') {
       promiseQueue.push(async () => {
         const browserPath = vfs.pathJoin(pkgDir, fieldValue)
-        await readFile(browserPath, currentPath, ['.js', '/index.js'])
+        const browserContent = await readFile(browserPath, currentPath, [
+          '.js',
+          '/index.js',
+        ])
+        if (browserContent === false) return
+        if (!isFileContentLintable(browserContent)) return
+        const expectFormat = await getFilePathFormat(browserPath, vfs)
+        // This check only matters if bundlers uses the file.
+        // If exports field is specified, bundlers will prefer that over browser field
+        if (
+          !hasExports &&
+          expectFormat === 'CJS' &&
+          hasEsModuleAndExportsDefault(browserContent)
+        ) {
+          messages.push({
+            code: 'CJS_WITH_ESMODULE_DEFAULT_EXPORT',
+            args: { filePath: '/' + vfs.pathRelative(pkgDir, browserPath) },
+            path: currentPath,
+            type: 'warning',
+          })
+        }
       })
     } else if (typeof fieldValue === 'object') {
       for (const key in fieldValue) {
-        crawlBrowser(fieldValue[key], currentPath.concat(key))
+        if (typeof fieldValue[key] === 'string') {
+          promiseQueue.push(async () => {
+            const browserPath = vfs.pathJoin(pkgDir, fieldValue[key])
+            await readFile(browserPath, currentPath.concat(key), [
+              '.js',
+              '/index.js',
+            ])
+          })
+        }
       }
     }
+
+    // if the package has both the `browser` and `exports` fields, recommend to use
+    // the browser condition instead
+    if (hasExports) {
+      if (typeof fieldValue === 'string') {
+        messages.push({
+          code: 'USE_EXPORTS_BROWSER',
+          args: {},
+          path: currentPath,
+          type: 'suggestion',
+        })
+      } else {
+        messages.push({
+          code: 'USE_EXPORTS_OR_IMPORTS_BROWSER',
+          args: {},
+          path: currentPath,
+          type: 'suggestion',
+        })
+      }
+    }
   }
@@ -688,6 +745,13 @@
   /**
+   * @typedef {{
+   *   isAfterNodeCondition: boolean,
+   *   isReachableByImportCondition: boolean,
+   * }} CrawlExportsOrImportsState
+   */
+
+  /**
    * @param {any} exportsValue
    * @param {string[]} currentPath
    * @param {boolean} isImports
-   * @param {boolean} isAfterNodeCondition
+   * @param {CrawlExportsOrImportsState} [state]
    */
@@ -698,3 +762,3 @@ function crawlExportsOrImports(
     isImports = false,
-    isAfterNodeCondition = false,
+    state,
   ) {
@@ -832,8 +896,22 @@ if (typeof exportsValue === 'string') {
             }
+            const expectFormat = await getFilePathFormat(filePath, vfs)
+            // This check only matters if it can be reached by `import`.
+            if (
+              (!state || state?.isReachableByImportCondition) &&
+              expectFormat === 'CJS' &&
+              hasEsModuleAndExportsDefault(fileContent)
+            ) {
+              messages.push({
+                code: 'CJS_WITH_ESMODULE_DEFAULT_EXPORT',
+                args: { filePath: '/' + vfs.pathRelative(pkgDir, filePath) },
+                path: currentPath,
+                type: 'warning',
+              })
+            }
             // file format checks isn't required for `browser` condition or exports
             // after the node condition, as nodejs doesn't use it, only bundlers do,
             // which doesn't care of the format
-            if (isAfterNodeCondition || currentPath.includes('browser')) return
+            if (state?.isAfterNodeCondition || currentPath.includes('browser'))
+              return
             const actualFormat = getCodeFormat(fileContent)
-            const expectFormat = await getFilePathFormat(filePath, vfs)
             if (
@@ -903,3 +981,3 @@ actualFormat !== expectFormat &&
           isImports,
-          isAfterNodeCondition,
+          state,
         )
@@ -1002,7 +1080,24 @@ }
 
-      // keep special state of whether the next `crawlExportsOrImports` iterations are after a node condition.
+      const isImportRequireConditions =
+        exportsKeys.includes('require') || exportsKeys.includes('import')
+
+      // whether the next `crawlExportsOrImports` iterations are after a node condition.
       // if there are, we can skip code format check as nodejs doesn't touch them, except bundlers
       // which are fine with any format.
-      let isKeyAfterNodeCondition = isAfterNodeCondition
+      // we track this outside of the for loop as we want the state to persist between the top-level keys.
+      let isAfterNodeCondition = state?.isAfterNodeCondition ?? false
+
       for (const key of exportsKeys) {
+        /** @type {CrawlExportsOrImportsState} */
+        const stateForNextLevel = {
+          isAfterNodeCondition,
+          isReachableByImportCondition:
+            state?.isReachableByImportCondition ?? true,
+        }
+        if (isImportRequireConditions) {
+          stateForNextLevel.isReachableByImportCondition &&=
+            key === 'import' ||
+            (!exportsKeys.includes('import') && key === 'default')
+        }
+
         // Check that import starts with `#`
@@ -1024,6 +1119,6 @@ if (isCurrentPathImports && !key.startsWith('#')) {
           isImports,
-          isKeyAfterNodeCondition,
+          stateForNextLevel,
         )
         if (key === 'node') {
-          isKeyAfterNodeCondition = true
+          isAfterNodeCondition = true
         }
@@ -1030,0 +1125,0 @@ }

src/shared/message.js

@@ -240,2 +240,11 @@ import picocolors from 'picocolors'
       return `${start} does not start with "${h.bold('#')}". Use ${h.bold(m.args.suggestKey)} instead.`
+    case 'CJS_WITH_ESMODULE_DEFAULT_EXPORT': {
+      const start =
+        m.path[0] === 'name' && m.args.filePath
+          ? h.bold(m.args.filePath)
+          : opts.reference
+            ? 'This file'
+            : `${h.bold(fp(m.path))}`
+      return `${start} sets both ${h.bold('exports.__esModule = true')} and ${h.bold('exports.default')}. This pattern breaks default imports when importing from an ESM-interpreted file in some bundlers. Consider using ESM syntax instead of CommonJS with ${h.bold('__esModule')} or avoid the default export.`
+    }
     default:
@@ -242,0 +251,0 @@ return

src/shared/utils.js

@@ -38,2 +38,32 @@ import { lintableFileExtensions } from './constants.js'
 
+// Check for problematic __esModule + exports.default pattern (https://github.com/publint/publint/issues/187)
+const EXPORTS___ESMODULE_PATTERN_RE =
+  /exports\.__esModule\s*=\s*true|Object\.defineProperty\s*\(\s*exports\s*,\s*["']__esModule["']\s*,\s*\{\s*value\s*:\s*true\s*\}/
+const EXPORTS_DEFAULT_PATTERN_RE = /exports\.default\s*=\s*([^\s]{7}.)/g
+/**
+ * @param {string} code
+ */
+export function hasEsModuleAndExportsDefault(code) {
+  const strippedCode = stripComments(code)
+  if (!EXPORTS___ESMODULE_PATTERN_RE.test(strippedCode)) {
+    return false
+  }
+  for (const match of strippedCode.matchAll(EXPORTS_DEFAULT_PATTERN_RE)) {
+    const partialDefaultValue = match[1].trimEnd()
+    if (
+      partialDefaultValue === 'exports' ||
+      partialDefaultValue === 'exports;'
+    ) {
+      // ignore `exports.default = exports`
+      continue
+    }
+    if (partialDefaultValue.startsWith('{')) {
+      // ignore `exports.default = { ... }`
+      continue
+    }
+    return true
+  }
+  return false
+}
+
 const MULTILINE_COMMENTS_RE = /\/\*(.|[\r\n])*?\*\//gm
@@ -40,0 +70,0 @@ const SINGLELINE_COMMENTS_RE = /\/\/.*/g

No alert changes

Improved metrics

Total package byte prevSize

107472

4.79%

Lines of code

2863

4.76%

No dependency changes