FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(scripts): close default-export bypass in dist verification by hannoeru · Pull Request #5312 · unocss/unocss · GitHub

/ unocss Public

fix(scripts): close default-export bypass in dist verification - #5312

Draft
hannoeru wants to merge 3 commits into
mainfrom
fix-dist-verification
Draft

hannoeru wants to merge 3 commits into
mainfrom
fix-dist-verification

Conversation

hannoeru commented Sep 6, 2026
edited
Loading

Copy link
Copy Markdown
Member

Problem

The postbuild dist checker (scripts/dist-verify.ts) could report success while a forbidden import was present. Its forbidden-require() rule was skipped for any file containing as default, so a declaration like:

import uno = require("@unocss/core")
export { uno as default }

passed validation. The script also carried obsolete commented-out checks, misleading cjsFiles naming, and had no test coverage.

Changes

  • Forbidden require() of @unocss/* in CJS declaration output (.d.ts / .d.cts) now always fails the build — the as default exemption is removed.
  • Scope intentionally limited to declaration outputs: runtime .cjs may require() ESM-only packages because Node ≥ 22.12 (the minimum engine) supports require(esm). The nuxt and webpack integrations rely on this; an earlier draft that scanned runtime output produced false positives in CI.
  • Removed obsolete commented-out implementation; renamed cjsFiles → cjsDeclarationFiles so logs match what is scanned.
  • Direct-execution guard now compares via pathToFileURL(), fixing a silent no-op on Windows (argv[1] is C:\..., never equal to a /C:/ URL pathname).
  • Added test/dist-verify.test.ts: spawns the real checker against temporary fixtures and asserts pass/fail exit codes, including the default-export case.

Validation

  • pnpm exec vitest run test/dist-verify.test.ts — 3/3 passed
  • Full checker run against a complete local build: Dist files verify passed
  • CI green across ubuntu/macos/windows matrices

dist-verify.ts scanned only declaration files while calling them cjsFiles,
kept obsolete commented-out checks, and exempted any file containing
'as default' from the forbidden require() check, so a forbidden import of
an ESM-only UnoCSS package could pass silently.

Scan the real CJS outputs (dist/**/*.cjs and dist/**/*.d.cts), drop the
obsolete commented implementation and the default-export exemption, and
cover the checker with tests that run it on fixture trees.

netlify Bot commented Sep 6, 2026
edited
Loading

Copy link
Copy Markdown

Deploy Preview for unocss ready!

Name Link
🔨 Latest commit c841c0b
🔍 Latest deploy log https://app.netlify.com/projects/unocss/deploys/6a9cdb345a8dd90007426cad
😎 Deploy Preview https://deploy-preview-5312--unocss.netlify.app
📱 Preview on mobile Toggle QR Code...



Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Runtime .cjs may require() ESM-only packages on Node >= 22.12
(require of ESM), so scanning runtime output produced false
positives for the nuxt and webpack integrations. Declarations
have no such escape hatch because no CJS types ship.

pkg-pr-new Bot commented Sep 6, 2026
edited
Loading

Copy link
Copy Markdown

Open in StackBlitz

commit: c841c0b

process.argv[1] is a Windows path while import.meta.url is a file
URL with a /C:/ prefix, so the direct-execution guard never matched
and the checker exited silently on Windows.
hannoeru changed the title fix(scripts): make dist verification fail on forbidden imports fix(scripts): close default-export bypass in dist verification Sep 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL