Feat: Integrate OXC for faster builds and improved Mincho transformations#281
Feat: Integrate OXC for faster builds and improved Mincho transformations#281sangkukbae wants to merge 15 commits intomainfrom
Conversation
|
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant Source as Client (TSX file)
participant VitePlugin as Mincho Vite Plugin
participant Env as Env Toggle (MINCHO_USE_BABEL)
participant OXC as `@mincho-js/oxc`
participant Babel as Babel backend
participant Esbuild as esbuild (child)
participant CSS as Virtual CSS Store
Source->>VitePlugin: transform(id, code)
VitePlugin->>Env: check MINCHO_USE_BABEL
alt MINCHO_USE_BABEL == true
VitePlugin->>Babel: delegate transform hooks
Babel->>Source: transformed code + virtual CSS artifacts
else
VitePlugin->>OXC: call oxcTransformFile / compileWithOxc
OXC->>Esbuild: run child compile (oxcBaseTransform)
Esbuild-->>OXC: compiled JS
OXC->>CSS: extract CSS -> generated extracted_*.css.ts
OXC-->>VitePlugin: transformed code + cssExtractions + watchFiles
VitePlugin->>Source: transformed code referencing virtual CSS
end
sequenceDiagram
participant Input as Source (TSX)
participant Phase1 as oxcBaseTransform
participant Phase2 as minchoTransform
participant Output as Result Code
participant Extractions as cssExtractions
Input->>Phase1: strip types, normalize JSX
Phase1-->>Phase2: plain JS
Phase2->>Phase2: find styled()/style(), replace with $$styled(), optionally generate extracted CSS modules
Phase2-->>Output: code with $$styled imports and virtual CSS imports
Phase2-->>Extractions: list of generated CSS modules and contents
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit 3a5e15ae8fdac7cf1a76111334716209df69323e (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Fri Oct 31 00:54:35 2025 +0900
Feat: Integrate OXC for faster builds and improved Mincho transformationsIt is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (22)
packages/oxc/CHANGELOG.md (1)
5-11: Consider softening “replaces Babel” wording for accuracySince the broader repo still supports Babel-based flows, you might want to clarify that this package introduces an OXC-based transform path (with feature parity) rather than fully deprecating/removing Babel across Mincho. E.g., “Introduces an OXC-based integration for Mincho.js, providing 10–15x faster builds with feature parity with the existing Babel implementation.”
packages/vite/src/utils.ts (1)
5-36: Utility implementations match stated behavior
extractedCssFileFiltercleanly captures “extracted_.css.ts” and “extracted_.css.ts?used” without allowing nested paths, which fits the documented intent.customNormalizeis straightforward and appropriate for normalizing Vite-style leading slashes. Adding a couple of unit tests for tricky paths (e.g. additional query params or unexpected slashes) would help prevent regressions, but the current logic looks sound.examples/react-oxc/eslint.config.js (1)
1-32: Refactor ESLint config to align with existing patternsThe codebase consistently spreads TypeScript ESLint configs at the top level of
defineConfig(). Inconfigs/eslint-config-custom/eslint.config.typescript.js,examples/react-swc/eslint.config.js, andexamples/react-babel/eslint.config.js, all use...tseslint.configs.recommended(or similar variants) directly in thedefineConfig()call.The file under review uses
extendswithtseslint.configs.recommendedinside a nested config object (line 16–21), which deviates from this pattern. To maintain consistency, refactor to spread the configs at the top level:export default defineConfig([ globalIgnores(['dist']), ...tseslint.configs.recommended, js.configs.recommended, // ... other configs { files: ['**/*.{ts,tsx}'], // ... rest of config }, ])packages/oxc/src/global.d.ts (1)
5-71: Ensure ambient OXC typings stay aligned with the realoxc-transform/oxc-parserAPIsThese declarations look reasonable, but they hard-code specific option shapes (e.g., a narrow
targetunion and minimalParseOptions/ParseErrorfields). If OXC adds new targets or options, these typings will drift and cause type errors even though runtime calls are valid.Consider either:
- Mirroring the official OXC TypeScript types (if exposed) instead of re-declaring shapes here, or
- Loosening the declarations (e.g.,
target?: string;and/or index signatures on options) so minor upstream changes don’t break consumers.Please cross-check this file against the OXC docs or published typings to confirm the current API surface.
packages/oxc/src/__tests__/base-transform.test.ts (2)
14-22: Consider strengthening JSX transformation assertions.The test only verifies that output is defined and non-empty, but doesn't validate the actual JSX transformation. For more robust testing, consider checking for specific patterns like
jsx(or React import statements.Example improvement:
it("should transform JSX", () => { const code = `const el = <div>Hello</div>;`; const result = oxcBaseTransform("test.tsx", code); expect(result.code).toBeDefined(); - // With automatic runtime, JSX transforms to jsx() calls - expect(result.code.length).toBeGreaterThan(0); + // With automatic runtime, JSX transforms to jsx() calls + expect(result.code).toMatch(/jsx|jsxs|jsxDEV/); });
39-52: Test name doesn't match assertions.The test is named "should preserve comments" but only verifies that variables
xandyexist, not that comments are actually preserved. Either rename the test to reflect what it checks or add assertions for comment preservation.Consider one of these approaches:
-it("should preserve comments", () => { +it("should handle code with comments", () => { const code = ` // This is a comment const x = 1; /* Block comment */ const y = 2; `; const result = oxcBaseTransform("test.ts", code); - // Comments might be preserved depending on OXC settings expect(result.code).toContain("x"); expect(result.code).toContain("y"); });Or if comment preservation is important:
expect(result.code).toContain("// This is a comment"); expect(result.code).toContain("/* Block comment */");packages/oxc/src/base-transform.ts (1)
16-44: Base transform defaults and options wiring look solidThe wrapper correctly threads
MinchoOxcBaseOptionsintooxc-transformwith sensible defaults (TS/JSX, target, sourcemap) and leaves Mincho‑specific work to the higher layer. One optional improvement would be to returnresultdirectly instead of{ code, map }if you ever need additional metadata fromTransformResult; for now the narrowed return is fine given current usage.packages/oxc/src/__tests__/mincho-transform.test.ts (1)
4-131: Good coverage of minchoTransform behavior, with a small tightening opportunityThe suite nicely exercises transformation, CSS extraction, ignore comments, multiple calls, empty input, and source maps. In the “unique CSS file names” test you guard the assertion with
if (result1.cssExtractions.length > 0 && result2.cssExtractions.length > 0). Given other tests already assert that simplestyle()calls produce extractions, you could drop the guard and assertlength > 0here as well to catch regressions earlier—but that’s optional.packages/vite/src/vite-babel.ts (2)
45-56: Guard against missingimporterinresolveIdfor extracted CSS
resolveIdassumesimporteris always defined whenextractedCssFileFilter(id)matches and usesjoin(importer!, "..", normalizedId). While that’s likely in normal import flows, defensive code would avoid potential runtime issues if Vite (or another plugin) ever calls this withimporterundefined.You can make this more robust with an early return:
if (extractedCssFileFilter(id)) { if (!importer) return; const normalizedId = id.startsWith("/") ? id.slice(1) : id; const resolvedPath = normalizePath(join(importer, "..", normalizedId)); ... }
19-44: Overall plugin structure is sound; minor naming and state‑handling tweaks optionalThe plugin wiring (maps for CSS contents and resolvers, handling of extracted CSS and
.vanilla.css, HMR invalidation, and delegation tobabelTransform/compile) follows a sensible pattern and is easy to follow.A couple of optional polish ideas:
- The plugin is named
"mincho-css-vite"here while there is now also an OXC‑based plugin; giving this a more explicit name like"mincho-css-vite-babel"could make debugging plugin order easier when both are in play.- In the JS/TS transform branch, you already clear and repopulate
idToPluginDataandresolverCache; adding a short comment explaining the lifecycle (JS file → extracted CSS path → CSS pluginData) would help future maintainers reason about the maps.Functionally this looks good once the
processVanillaFilereturn handling is confirmed/fixed.Also applies to: 67-74, 98-107, 113-229
packages/integration/src/__tests__/compile-oxc.test.ts (1)
8-305: Strong coverage for compileWithOxc; only a couple of brittle expectationsThe suite exercises most important paths (plain TS, styled(), JSX, cache, externals, Babel parity, complex objects) and the temp project scaffolding is solid. The only minor fragility is that a few assertions are tightly coupled to current output shapes (e.g. checking for the literal
"jsx"substring andrequire("some-external-package")). If you later tweak JSX runtime details or esbuild format/platform settings, those specific expectations may need adjustment; otherwise the structure of the tests looks good.packages/integration/src/compile-oxc.ts (2)
34-59: Reuse resolverCache inside the esbuild plugin to avoid redundant addFileScope + I/OYou already maintain
resolverCachefororiginalPath, but theonLoadhook always re-reads and re-scopes every TS/JS/TSX/JSX file. IfcompileWithOxcis used repeatedly (or if esbuild loads the same path multiple times), you could avoid extrareadFile+addFileScopework by sharing that cache in the plugin as well.A minimal change is to look up
args.pathinresolverCachebefore reading:{ name: "mincho:oxc-transform", setup(build) { build.onLoad({ filter: /\.(t|j)sx?$/ }, async (args) => { - const contents = await fs.promises.readFile(args.path, "utf-8"); - - // All files need addFileScope - const source = addFileScope({ - source: contents, - filePath: args.path, - rootPath: build.initialOptions.absWorkingDir!, - packageName: packageInfo.name - }); + let source = resolverCache.get(args.path); + + if (!source) { + const contents = await fs.promises.readFile(args.path, "utf-8"); + + // All files need addFileScope + source = addFileScope({ + source: contents, + filePath: args.path, + rootPath: build.initialOptions.absWorkingDir!, + packageName: packageInfo.name + }); + + resolverCache.set(args.path, source); + }This keeps behavior identical while reducing filesystem and addFileScope work for repeated builds.
Also applies to: 82-96
135-140: Make watchFiles path construction robust to absolute metafile inputsCurrently
watchFilesis built with:watchFiles: Object.keys(metafile?.inputs || {}).map((filePath) => join(cwd, filePath) )If any key in
metafile.inputsis already absolute,join(cwd, filePath)can produce an invalid combined path. It’s safer to preserve absolute inputs as-is and only join relative ones:-import { basename, dirname, join } from "path"; +import { basename, dirname, join, isAbsolute } from "path"; @@ - watchFiles: Object.keys(metafile?.inputs || {}).map((filePath) => - join(cwd, filePath) - ) + watchFiles: Object.keys(metafile?.inputs || {}).map((filePath) => + isAbsolute(filePath) ? filePath : join(cwd, filePath) + )This keeps existing behavior for relative entries while avoiding malformed paths if esbuild ever reports absolute ones.
packages/vite/src/types.ts (1)
5-38: Vite type shims are intentionally minimal; consider documenting or aligning if exposed publiclyThese interfaces are a slim subset of Vite’s actual types (e.g. synchronous hooks only,
getModuleByIdnever undefined,build.watchas a boolean flag). That’s fine for internal use, but ifPlugin/ViteDevServer/ResolvedConfigare part of your public surface, it may surprise consumers expecting drop‑in Vite compat.Two low-friction options:
- Add a brief comment noting these are narrowed shims, not full Vite typings, or
- In
.d.tsonly, alias to Vite’s ownPlugin/ViteDevServertypes to avoid drift.packages/vite/src/index.ts (1)
30-46: Clarify options shape to avoid silent misconfiguration when MINCHO_USE_BABEL is setThe env gate works, but with the current union + casts:
options?: | { babel?: BabelOptions } | { oxc?: MinchoOxcBaseOptions; mincho?: { extractCSS?: boolean } }if someone passes OXC options and sets
MINCHO_USE_BABEL="true", those options are still forwarded tominchoBabelVitePluginbut ignored (it only looks at.babel). The inverse is also possible if you later add more branches.To make misconfigurations more obvious and reduce casting, consider:
A single options type like:
type MinchoViteOptions = { babel?: BabelOptions; oxc?: MinchoOxcBaseOptions; mincho?: { extractCSS?: boolean }; };and pick the relevant subset in each branch, or
A discriminated union with an explicit mode, e.g.
{ engine: "babel"; babel?: ... } | { engine: "oxc"; oxc?: ... }, so TS can catch mismatched usage.Not urgent, but it would make the API easier to reason about.
packages/integration/src/oxc.ts (1)
19-54: Deduplicate shared OXC transform pipeline between oxcTransformFile and oxcTransformBoth functions run the same two-phase pipeline (oxcBaseTransform → minchoTransform) and build the same result shape;
oxcTransformFilejust adds a file read. To avoid drift and make future changes safer, you can haveoxcTransformFiledelegate tooxcTransformafter reading the file (and share an options type):-import type { - MinchoOxcBaseOptions, - MinchoTransformOptions -} from "@mincho-js/oxc"; +import type { + MinchoOxcBaseOptions, + MinchoTransformOptions +} from "@mincho-js/oxc"; + +type OxcTransformOptions = { + oxc?: MinchoOxcBaseOptions; + mincho?: Partial<MinchoTransformOptions>; + cwd?: string; +}; @@ export async function oxcTransformFile( path: string, - options?: { - oxc?: MinchoOxcBaseOptions; - mincho?: Partial<MinchoTransformOptions>; - cwd?: string; - } + options?: OxcTransformOptions ) { - const cwd = options?.cwd || process.cwd(); - - const code = await readFile(path, "utf-8"); - - // Phase 1: OXC base transform (TS/JSX only) - const oxcResult = oxcBaseTransform(path, code, options?.oxc); - - // Phase 2: Mincho custom transform - const minchoResult = minchoTransform(oxcResult.code, { - filename: path, - sourceRoot: cwd, - extractCSS: true, - ...options?.mincho - }); - - return { - code: minchoResult.code, - result: - minchoResult.cssExtractions.length > 0 - ? [ - minchoResult.cssExtractions[0].id, - minchoResult.cssExtractions[0].content - ] - : ["", ""], - cssExtractions: minchoResult.cssExtractions, - dependencies: minchoResult.dependencies - }; + const code = await readFile(path, "utf-8"); + return oxcTransform(path, code, options); } @@ export function oxcTransform( filename: string, code: string, - options?: { - oxc?: MinchoOxcBaseOptions; - mincho?: Partial<MinchoTransformOptions>; - cwd?: string; - } + options?: OxcTransformOptions ) {This keeps behavior identical while centralizing the transform logic.
Also applies to: 64-98
packages/oxc/src/types.ts (1)
21-30: UnifycssExtractionselement type withCSSExtraction
MinchoTransformResult.cssExtractionsinlines a{ id; content; dependencies }shape that duplicates theCSSExtractioninterface below. Consider reusing the named type for consistency and easier evolution:-export interface MinchoTransformResult { - code: string; - map: any; - cssExtractions: Array<{ - id: string; - content: string; - dependencies: string[]; - }>; - dependencies: string[]; -} +export interface MinchoTransformResult { + code: string; + map: any; + cssExtractions: CSSExtraction[]; + dependencies: string[]; +}Also applies to: 54-58
packages/vite/src/vite-oxc.ts (3)
22-30: Several plugin options are currently unused (include/exclude/virtualModulePrefix)
MinchoOxcVitePluginOptionsexposesinclude,exclude, andmincho.virtualModulePrefix, but the implementation only readsoxcandmincho.extractCSS. This can confuse users expecting these options to work (especially parity with the Babel plugin).Either wire these into the transform flow (e.g., via a filter on
idintransform()/load()), or document/remove them from the public API until supported.Also applies to: 42-52, 203-230
219-230: Preserve or propagate source maps from OXC for better DXThe
oxcTransformFileresult appears to be discarded except forcode, and the plugin returns a dummy map{ mappings: "" }. This loses potentially useful sourcemap information for debugging and HMR.If
oxcTransformFile(and downstream transforms) can provide a real sourcemap, consider threading it through here, falling back tonullwhen unavailable:- const { - code: transformedCode, - result: [file, cssExtract], - cssExtractions - } = await oxcTransformFile(id, { + const { + code: transformedCode, + map, + result: [file, cssExtract], + cssExtractions + } = await oxcTransformFile(id, { oxc: _options?.oxc, mincho: { extractCSS: _options?.mincho?.extractCSS ?? true }, cwd: config.root }); ... - return { - code: transformedCode, - map: { mappings: "" } - }; + return { + code: transformedCode, + map: map ?? null + }; ... - return { - code: transformedCode, - map: { mappings: "" } - }; + return { + code: transformedCode, + map: map ?? null + };Also applies to: 250-255, 290-293
100-121: Clarify lifecycle and contents ofidToPluginDatafor extracted CSS filesThe
load()andtransform()branches for extracted CSS files rely onidToPluginDataentries keyed by both a normalized path and the rawid, andload()overwrites theidentry with only{ filePath, originalPath }. This is probably fine giventransform()uses only those two fields, but the pattern is subtle and easy to regress.Consider:
- Basing the
set(id, ...)call onpluginDatainstead ofidToPluginData.get(id)(currently alwaysundefinedon first load), and- Adding a short comment near the
idToPluginData.set(id, ...)call explaining why onlyfilePath/originalPathare needed later.Example:
- idToPluginData.set(id, { - ...idToPluginData.get(id), - filePath: id, - originalPath: id // Use the extracted file path itself as originalPath - }); + idToPluginData.set(id, { + ...pluginData, + filePath: id, + originalPath: id // Use the extracted file path itself as originalPath + });This keeps the data flow more obvious without changing behavior.
Also applies to: 140-158
packages/oxc/src/mincho-transform.ts (2)
176-195: Dynamic RegExp is safe here but could be constrained furtherStatic analysis flags
new RegExp(\\b${functionName}\s*\(`, "g")as “regexp-from-variable”. In this file,functionNameis only ever passed as the literal"styled"` (and not user input), so ReDoS is not a concern in practice.To make this intent clearer and silence future tooling complaints, you could either:
- Inline a fixed pattern (e.g.
const regex = /\bstyled\s*\(/g;), or- Restrict usage of
findFunctionCallsto known-safe, internal call sites only (as it is today).Functionally, the current code is acceptable; this is mostly about clarity and static-analysis friendliness.
200-256: Helper scanners are intentionally approximate; consider documenting limitationsThe helpers
findMatchingClosingParen,findFirstCommaOutsideParens, andfindMatchingBraceare manually implemented scanners that handle common string and template-literal cases but are not full JS parsers. For complex expressions (nested template literals, unusual formatting), they may fail and cause the transform to skip astyled()call.Given this is by design to keep the transform fast and lightweight, consider adding a short comment near each helper (or above
minchoTransform) noting that:
- They are best-effort heuristics, and
- When they fail, the call is intentionally left untransformed (which should be consistent with the import-removal logic discussed above).
This will help future maintainers understand the tradeoffs and avoid overextending these helpers without tests.
Also applies to: 261-300, 305-360
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
examples/react-oxc/public/vite.svgis excluded by!**/*.svgexamples/react-oxc/src/assets/react.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (41)
examples/react-babel/vite.config.ts(0 hunks)examples/react-oxc/.gitignore(1 hunks)examples/react-oxc/README.md(1 hunks)examples/react-oxc/eslint.config.js(1 hunks)examples/react-oxc/index.html(1 hunks)examples/react-oxc/src/App.tsx(1 hunks)examples/react-oxc/src/main.tsx(1 hunks)examples/react-oxc/tsconfig.app.json(1 hunks)examples/react-oxc/tsconfig.json(1 hunks)examples/react-oxc/tsconfig.node.json(1 hunks)examples/react-oxc/vite.config.ts(1 hunks)packages/css/tsconfig.json(1 hunks)packages/css/tsconfig.lib.json(1 hunks)packages/integration/package.json(1 hunks)packages/integration/src/__tests__/benchmark.test.ts(1 hunks)packages/integration/src/__tests__/compile-oxc.test.ts(1 hunks)packages/integration/src/compile-oxc.ts(1 hunks)packages/integration/src/compile.ts(4 hunks)packages/integration/src/index.ts(1 hunks)packages/integration/src/oxc.ts(1 hunks)packages/oxc/CHANGELOG.md(1 hunks)packages/oxc/README.md(1 hunks)packages/oxc/eslint.config.js(1 hunks)packages/oxc/src/__tests__/base-transform.test.ts(1 hunks)packages/oxc/src/__tests__/mincho-transform.test.ts(1 hunks)packages/oxc/src/base-transform.ts(1 hunks)packages/oxc/src/global.d.ts(1 hunks)packages/oxc/src/index.ts(1 hunks)packages/oxc/src/mincho-transform.ts(1 hunks)packages/oxc/src/types.ts(1 hunks)packages/oxc/src/utils.ts(1 hunks)packages/oxc/tsconfig.json(1 hunks)packages/oxc/tsconfig.lib.json(1 hunks)packages/oxc/tsconfig.node.json(1 hunks)packages/oxc/vite.config.ts(1 hunks)packages/vite/package.json(1 hunks)packages/vite/src/index.ts(1 hunks)packages/vite/src/types.ts(1 hunks)packages/vite/src/utils.ts(1 hunks)packages/vite/src/vite-babel.ts(1 hunks)packages/vite/src/vite-oxc.ts(1 hunks)
💤 Files with no reviewable changes (1)
- examples/react-babel/vite.config.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-09T13:00:38.449Z
Learnt from: black7375
Repo: mincho-js/mincho PR: 113
File: packages/css/src/index.ts:33-33
Timestamp: 2024-10-09T13:00:38.449Z
Learning: `recipe` is properly exported from './rules' in `packages/css/src/rules/index.ts`.
Applied to files:
packages/oxc/tsconfig.jsonpackages/integration/src/compile.ts
📚 Learning: 2024-12-03T11:16:32.528Z
Learnt from: black7375
Repo: mincho-js/mincho PR: 134
File: packages/transform-to-vanilla/src/transform.ts:49-51
Timestamp: 2024-12-03T11:16:32.528Z
Learning: In projects that target both CommonJS and ES modules, adjusting the TypeScript `module` option may not be feasible. Therefore, using `ts-ignore` comments to suppress TypeScript errors related to `import.meta` is acceptable.
Applied to files:
packages/oxc/eslint.config.jsexamples/react-oxc/eslint.config.js
🧬 Code graph analysis (18)
packages/oxc/src/__tests__/base-transform.test.ts (2)
packages/oxc/src/base-transform.ts (1)
oxcBaseTransform(16-44)packages/oxc/src/index.ts (1)
oxcBaseTransform(6-6)
packages/oxc/tsconfig.lib.json (1)
configs/vite-config-custom/src/index.ts (1)
NodeBuilder(45-128)
examples/react-oxc/tsconfig.node.json (1)
configs/vite-config-custom/src/index.ts (2)
NodeConfig(24-29)NodeBuilder(45-128)
packages/integration/src/compile.ts (1)
packages/esbuild/src/index.ts (1)
minchoEsbuildPlugin(25-131)
packages/oxc/tsconfig.node.json (1)
configs/vite-config-custom/src/index.ts (2)
NodeConfig(24-29)NodeBuilder(45-128)
packages/integration/src/__tests__/benchmark.test.ts (2)
packages/integration/src/compile.ts (1)
compile(18-109)packages/integration/src/compile-oxc.ts (1)
compileWithOxc(34-141)
packages/oxc/vite.config.ts (1)
configs/vite-config-custom/src/index.ts (1)
NodeConfig(24-29)
packages/integration/src/__tests__/compile-oxc.test.ts (2)
packages/integration/src/compile-oxc.ts (1)
compileWithOxc(34-141)packages/integration/src/compile.ts (1)
compile(18-109)
examples/react-oxc/src/App.tsx (3)
packages/react/src/index.ts (2)
styled(145-145)styledImpl(95-117)packages/react/src/runtime.ts (1)
$$styled(17-81)packages/babel/src/styled.ts (1)
styledComponentPlugin(14-92)
examples/react-oxc/vite.config.ts (2)
packages/vite/src/index.ts (1)
minchoVitePlugin(30-47)configs/vite-config-custom/src/index.ts (2)
NodeConfig(24-29)NodeBuilder(45-128)
packages/oxc/CHANGELOG.md (2)
packages/esbuild/src/index.ts (1)
minchoEsbuildPlugin(25-131)packages/babel/src/index.ts (1)
minchoBabelPlugin(8-19)
packages/oxc/eslint.config.js (1)
configs/eslint-config-custom/eslint.config.typescript.js (1)
eslintConfig(19-71)
examples/react-oxc/eslint.config.js (1)
configs/eslint-config-custom/eslint.config.typescript.js (1)
eslintConfig(19-71)
packages/oxc/src/__tests__/mincho-transform.test.ts (2)
packages/oxc/src/index.ts (1)
minchoTransform(7-7)packages/oxc/src/mincho-transform.ts (1)
minchoTransform(15-171)
packages/integration/src/compile-oxc.ts (4)
packages/integration/src/index.ts (1)
compileWithOxc(4-4)packages/oxc/src/base-transform.ts (1)
oxcBaseTransform(16-44)packages/oxc/src/mincho-transform.ts (1)
minchoTransform(15-171)packages/esbuild/src/index.ts (2)
minchoEsbuildPlugin(25-131)args(101-128)
packages/css/tsconfig.lib.json (5)
packages/css/src/theme/types.ts (1)
compositeValue(199-203)packages/css/src/rules/compat-impl.ts (1)
CompatPatternOptions(51-66)packages/transform-to-vanilla/src/types/style-rule.ts (2)
CSSComplexProperties(52-54)CSSMergeProperties(147-149)packages/css/src/rules/types.ts (1)
PatternOptions(170-185)packages/css/src/theme/index.ts (1)
compositeValue(731-735)
packages/oxc/src/mincho-transform.ts (2)
packages/oxc/src/index.ts (1)
minchoTransform(7-7)packages/oxc/src/types.ts (2)
MinchoTransformOptions(35-39)MinchoTransformResult(21-30)
packages/vite/src/index.ts (5)
packages/integration/src/babel.ts (1)
BabelOptions(8-16)packages/oxc/src/types.ts (1)
MinchoOxcBaseOptions(4-16)packages/vite/src/types.ts (1)
Plugin(30-39)packages/vite/src/vite-babel.ts (1)
minchoBabelVitePlugin(19-233)packages/vite/src/vite-oxc.ts (1)
minchoOxcVitePlugin(42-298)
🪛 ast-grep (0.40.0)
packages/oxc/src/mincho-transform.ts
[warning] 180-180: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b${functionName}\\s*\\(, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (25)
packages/integration/src/compile.ts (3)
4-5: LGTM!The import additions are correctly used throughout the file—
basenameat line 49,fs.promises.readFileat line 69, anddirnameat lines 48 and 89.
69-77: LGTM—consistent file scope processing.Applying
addFileScopeto all files loaded in the esbuild plugin ensures consistent scoping for CSS extraction across the codebase. The comment at line 71 clearly documents this intent, and the approach mirrors the handling of the original path at lines 34-39.
53-60: Verified: subpath exports are valid.The subpath exports
@vanilla-extract/css/adapterand@vanilla-extract/css/fileScopeare both confirmed valid exports in the @vanilla-extract/css package. The externals configuration correctly prevents esbuild from bundling these packages. No issues found.packages/css/tsconfig.json (1)
3-5: Composite flag aligns with project referencesEnabling
"composite": truehere makes sense for a referenced project and matches the existing references block. Just ensure this tsconfig isn’t accidentally used for direct emits without matchingdeclaration/outDirsettings elsewhere, as TS can get picky about composite projects if emit is enabled.Also applies to: 13-18
examples/react-oxc/src/main.tsx (1)
1-9: React 18 entrypoint looks good
createRoot+StrictModeusage and the#rootbinding match the HTML template; the non-null assertion is acceptable in this controlled example context. Optionally, a runtime null-check with an explicit error could improve robustness, but not required here.examples/react-oxc/index.html (1)
1-13: HTML shell matches React entrypoint
<div id="root"></div>and the module script pointing to/src/main.tsxline up withmain.tsx. No changes needed.examples/react-oxc/tsconfig.app.json (1)
1-28: Modern TS bundler config looks consistent; confirm TS version supports all flagsThe options here (bundler resolution,
verbatimModuleSyntax,noEmit,jsx: "react-jsx") are a good fit for a Vite app. Flags likeerasableSyntaxOnlyandnoUncheckedSideEffectImportsrely on newer TypeScript versions, so just double-check the repo’s TS version aligns with these.examples/react-oxc/.gitignore (1)
1-24: Ignore patterns are appropriate for the example appThe ignores cover logs, build artifacts, and common editor/OS files; this is sufficient for the React + Vite example.
examples/react-oxc/README.md (1)
1-75: README content looks consistent with the React+OXC exampleThe React Compiler note, ESLint snippets, and tsconfig paths line up with the files in
examples/react-oxc; nothing seems misleading for consumers of this template.examples/react-oxc/src/App.tsx (1)
1-67: Sample App and styled usage look correctThe BaseComponent/Container composition, variant definitions (size/color), compoundVariants, and defaultVariants are coherent, and the
Apprender matches the defined variants. This is a good minimal example for exercising the OXC+Mincho pipeline.packages/css/tsconfig.lib.json (1)
4-4: Enablingcompositefor the css lib config is appropriateTurning on
compositehere matches the project-reference / dts generation strategy used elsewhere and should work well with the shared Vite/TS build pipeline.Please run your usual TypeScript build/check (e.g., workspace
checkscript) to confirm there are no new project-reference or incremental-build warnings.packages/vite/package.json (1)
55-63: Adding@mincho-js/oxcas a workspace dependency looks correctThis hooks the Vite plugin up to the new OXC-based transform implementation alongside the existing Babel path; no manifest issues stand out.
packages/oxc/tsconfig.node.json (1)
1-4: Node-only tsconfig forvite.config.tsis scoped appropriatelyExtending the shared node config and limiting
includetovite.config.tskeeps the Vite config typed without polluting the main project config.Please ensure your editor/TS server is picking up this
tsconfig.node.jsonfor the Vite config (or that your tooling is configured to use it).packages/oxc/tsconfig.lib.json (1)
1-8: Library tsconfig for@mincho-js/oxcmatches the shared build patternEnabling
composite/declaration, wiring atsBuildInfoFileunder.cache/typescript, and adding Node types is consistent with how other packages are configured and should integrate smoothly with the shared Vite/TS build pipeline. The exacttsbuildinfofilename differs slightly from css (tsbuildinfo-esmthere), but that’s only a naming convention, not a functional problem.After this change, it’s worth running the workspace build/check once to confirm the dts generation plugins that reference
tsconfig.lib.jsonbehave as expected.packages/integration/package.json (1)
58-58: LGTM!The workspace dependency addition is clean and consistent with the existing pattern for
@mincho-js/babel. This enables the OXC integration layer as described in the PR objectives.examples/react-oxc/tsconfig.json (1)
1-7: LGTM!This solution-style TypeScript configuration correctly uses project references to delegate to
tsconfig.app.jsonandtsconfig.node.json. The emptyfilesarray is the standard pattern for this approach.packages/oxc/tsconfig.json (1)
1-14: LGTM!The TypeScript configuration is well-structured:
- Properly excludes test files from compilation
- References the CSS package, which aligns with the OXC package's CSS extraction functionality
- Follows standard monorepo package configuration patterns
packages/integration/src/index.ts (1)
2-4: LGTM!The new exports cleanly extend the integration package's API with OXC functionality, following the same pattern as the existing Babel exports. This provides a consistent interface for both transformation approaches.
packages/oxc/README.md (1)
1-283: Excellent documentation!This README provides comprehensive coverage of the OXC package:
- Clear architecture explanation with two-phase transformation
- Practical usage examples for both standalone and Vite integration
- Detailed API documentation with TypeScript interfaces
- Before/after transformation examples
- Performance benchmarks
- Compatibility matrix
- Migration guidance and troubleshooting
The documentation will greatly help users adopt the OXC-based transformation workflow.
examples/react-oxc/tsconfig.node.json (1)
1-26: LGTM!This TypeScript configuration for Node/bundler tooling follows modern best practices:
- Uses bundler module resolution appropriate for Vite
- Enables strict type checking with comprehensive linting rules
- Properly scoped to only include
vite.config.ts- Aligns with the separation of concerns pattern (app vs. build tooling)
packages/oxc/src/index.ts (1)
1-9: Barrel exports look consistent with the new OXC API surfaceRe-exports are clear and keep the public surface small while exposing the main primitives (base transform, Mincho transform, shared types/utils). No issues spotted.
packages/integration/src/__tests__/benchmark.test.ts (1)
18-44: Benchmark helper is straightforward and side‑effect freeThe
measurePerformancehelper is well‑scoped (time + heap delta, optionalglobal.gc()), and doesn’t couple tests to any global state beyond process metrics. No changes needed here.packages/oxc/vite.config.ts (1)
1-12: Vite config follows existing NodeConfig patternUsing
NodeConfig(viteConfigEnv, { test: { name: "@mincho-js/oxc" } })is consistent with the other package configs and neatly scopes the Vitest project name. Looks good.packages/vite/src/vite-babel.ts (1)
75-107: The review comment is based on incorrect assumptions about theprocessVanillaFileAPI.According to the web search results, processVanillaFile returns the generated module source (a string), not an object with
{ source, watchFiles }. The current code at lines 130-150 is correct: it returnscontentsdirectly from thetransformhook, which is valid since Vite'stransformhook accepts either a string or an object with acodeproperty.The watch file handling is already properly implemented with
this.addWatchFile(file)on lines 125-131, so the suggested refactoring is unnecessary.Likely an incorrect or invalid review comment.
packages/oxc/src/types.ts (1)
1-40: Public OXC/Mincho option/result types look consistent and usableThe overall type surface (
MinchoOxcBaseOptions,MinchoTransformOptions,MinchoTransformResult,MinchoVisitors) is coherent and matches how the transform code uses them (e.g., onlyfilename/extractCSSare required at runtime). No functional issues here.
| // https://vite.dev/config/ | ||
| export default defineConfig({ | ||
| plugins: [ | ||
| minchoVitePlugin() as unknown as PluginOption, |
There was a problem hiding this comment.
🧩 Analysis chain
Investigate the double type assertion.
The as unknown as PluginOption cast suggests a type incompatibility between minchoVitePlugin()'s return type and Vite's PluginOption. Type assertions like this can mask type safety issues.
Please verify that minchoVitePlugin returns a type compatible with Vite's plugin system. If there's a type definition mismatch, consider:
- Updating the plugin's return type declaration
- Checking if the plugin wrapper needs adjustment
- Confirming this pattern works correctly at runtime
Run the following to check the plugin's type definition:
🏁 Script executed:
#!/bin/bash
# Check the return type of minchoVitePlugin
ast-grep --pattern $'export function minchoVitePlugin($$$): $RETURN_TYPE {
$$$
}'Length of output: 1128
I need to gather more information about the type definitions and context. Let me execute these scripts to verify the type compatibility and understand why the double cast is used.
🏁 Script executed:
#!/bin/bash
# Check the imports and type definitions in the vite package
cat packages/vite/src/index.ts | head -40Length of output: 1189
🏁 Script executed:
#!/bin/bash
# Look at the vite.config.ts file to understand the context
cat examples/react-oxc/vite.config.tsLength of output: 312
🏁 Script executed:
#!/bin/bash
# Search for Plugin import in the vite package
rg -n "import.*Plugin" packages/vite/src/index.ts -A2 -B2Length of output: 292
🏁 Script executed:
#!/bin/bash
# Check what Plugin type is being used/exported
rg -n "^import.*from.*vite" packages/vite/src/index.tsLength of output: 113
🏁 Script executed:
#!/bin/bash
# Find where PluginOption is used in the examples
rg -n "PluginOption" examples/react-oxc/vite.config.ts -B3 -A3Length of output: 339
🏁 Script executed:
#!/bin/bash
# Check the actual mincho vite plugin exports
rg -n "export.*Plugin" packages/vite/src/index.tsLength of output: 36
🏁 Script executed:
#!/bin/bash
# Look for re-exports or type definitions in the vite package
ls -la packages/vite/src/Length of output: 409
🏁 Script executed:
#!/bin/bash
# Check the package.json or tsconfig to understand the project structure
cat packages/vite/package.json | head -30Length of output: 799
🏁 Script executed:
#!/bin/bash
# Look at what minchoBabelVitePlugin and minchoOxcVitePlugin return
rg -n "minchoBabelVitePlugin|minchoOxcVitePlugin" packages/vite/src/index.ts -B5 -A5Length of output: 973
🏁 Script executed:
#!/bin/bash
# Check for Plugin type definition in Vite
rg -n "type Plugin|interface Plugin" packages/vite/src/ -A10Length of output: 846
🏁 Script executed:
#!/bin/bash
# Search for PluginOption in the codebase
rg -n "PluginOption" packages/vite/src/ -B2 -A2Length of output: 605
🏁 Script executed:
#!/bin/bash
# Check if Plugin is a type from Vite
rg -n "Plugin" packages/vite/src/index.ts -B1 -A1 | head -30Length of output: 834
🏁 Script executed:
#!/bin/bash
# Look at minchoBabelVitePlugin and minchoOxcVitePlugin definitions
fd "minchoBabel|minchoOxc" packages/vite/src/ --type f -x cat {}Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get the import statements from vite in the config
rg -n "^import" examples/react-oxc/vite.config.tsLength of output: 176
🏁 Script executed:
#!/bin/bash
# Check the mincho package.json to see vite version
rg "vite" packages/vite/package.jsonLength of output: 196
🏁 Script executed:
#!/bin/bash
# Look for type definitions in the mincho vite package more broadly
find packages/vite -name "*.ts" -o -name "*.d.ts" | xargs ls -laLength of output: 465
🏁 Script executed:
#!/bin/bash
# Check if there's a types file that re-exports Plugin
cat packages/vite/src/types.ts 2>/dev/null || echo "No types.ts found"Length of output: 855
🏁 Script executed:
#!/bin/bash
# Get all .ts files in the vite package
ls -la packages/vite/src/*.tsLength of output: 387
🏁 Script executed:
#!/bin/bash
# Check the full vite.config.ts file including imports
head -25 examples/react-oxc/vite.config.tsLength of output: 312
🏁 Script executed:
#!/bin/bash
# Look at the vite plugin type in Vite's own exports
rg -n "PluginOption" . --type ts -B2 -A2 | head -50Length of output: 2950
🏁 Script executed:
#!/bin/bash
# Find where Vite is imported from
rg "from ['\"]vite['\"]" examples/react-oxc/vite.config.tsLength of output: 76
🏁 Script executed:
#!/bin/bash
# Check the return type more carefully - look at minchoOxcVitePlugin
rg -n "export.*minchoOxcVitePlugin|function minchoOxcVitePlugin" packages/vite/src/Length of output: 69
🏁 Script executed:
#!/bin/bash
# Let's look at what's exported from the main vite plugin file
rg "export" packages/vite/src/index.ts | head -20Length of output: 99
🏁 Script executed:
#!/bin/bash
# Check all imports from 'vite' in the entire vite package
rg "from ['\"]vite['\"]" packages/vite/src/ -A1 -B1Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the actual plugin file structure
find packages/vite/src -type f -name "*.ts" -exec echo "=== {} ===" \; -exec head -5 {} \;Length of output: 684
🏁 Script executed:
#!/bin/bash
# Check mincho vite package structure
find packages/vite -maxdepth 2 -type f | grep -E "\.(ts|js|json)$"Length of output: 322
🏁 Script executed:
#!/bin/bash
# Look at the specific Plugin type used - is it Vite's Plugin?
cat packages/vite/src/index.ts | grep -E "Plugin|import" | head -40Length of output: 559
🏁 Script executed:
#!/bin/bash
# Get the complete first 60 lines of the vite package
head -60 packages/vite/src/index.tsLength of output: 1394
🏁 Script executed:
#!/bin/bash
# Check vite.config.ts completely
cat examples/react-oxc/vite.config.tsLength of output: 312
🏁 Script executed:
#!/bin/bash
# Look at vite version requirement
cat packages/vite/package.json | grep -A2 -B2 "dependencies\|devDependencies"Length of output: 274
🏁 Script executed:
#!/bin/bash
# Search for Vite imports across the entire packages/vite folder
find packages/vite/src -type f -name "*.ts" -exec grep -l "from.*vite" {} \;Length of output: 26
🏁 Script executed:
#!/bin/bash
# Get a comprehensive view of index.ts
wc -l packages/vite/src/index.ts && echo "---" && cat packages/vite/src/index.tsLength of output: 1428
🏁 Script executed:
#!/bin/bash
# Look at what minchoBabelVitePlugin actually returns
grep -n "minchoBabelVitePlugin" packages/vite/src/index.ts -A30 | head -50Length of output: 1300
🏁 Script executed:
#!/bin/bash
# Find all files in packages/vite/src and show their content
for file in packages/vite/src/*.ts; do echo "=== $file ==="; cat "$file"; doneLength of output: 18731
🏁 Script executed:
#!/bin/bash
# Simpler: just cat the babel file
cat packages/vite/src/babel.ts 2>/dev/null || echo "babel.ts not found"Length of output: 18
🏁 Script executed:
#!/bin/bash
# List all files in vite src
ls -la packages/vite/src/Length of output: 409
🏁 Script executed:
#!/bin/bash
# Show content of all TS files
for f in packages/vite/src/*.ts; do echo "### $(basename $f)"; cat "$f"; echo; doneLength of output: 18626
🏁 Script executed:
#!/bin/bash
# Check the oxc.ts file specifically
cat packages/vite/src/oxc.ts 2>/dev/null | head -100Length of output: 0
🏁 Script executed:
#!/bin/bash
# Try to find all imports from vite
rg -n "import.*from.*['\"]vite" packages/vite/Length of output: 149
🏁 Script executed:
#!/bin/bash
# Check exact structure
ls -la packages/vite/src/ && echo "---" && file packages/vite/src/*Length of output: 456
🏁 Script executed:
#!/bin/bash
# Look at imports more carefully
head -15 packages/vite/src/index.ts && echo "..." && grep -n "Plugin" packages/vite/src/index.ts | head -10Length of output: 941
🏁 Script executed:
#!/bin/bash
# Get mincho Vite package info
cat packages/vite/package.jsonLength of output: 2063
🏁 Script executed:
#!/bin/bash
# Check if there are type definitions elsewhere
find packages/vite -name "*.d.ts" -o -name "index.d.ts"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at vite dependency version
grep -E "\"vite\"" packages/vite/package.jsonLength of output: 10
🏁 Script executed:
#!/bin/bash
# Simple content check
cat packages/vite/src/index.ts 2>/dev/null | wc -lLength of output: 2
🏁 Script executed:
#!/bin/bash
# Try to get first 80 lines
sed -n '1,80p' packages/vite/src/index.tsLength of output: 1394
🏁 Script executed:
#!/bin/bash
# Get imports and Plugin references
grep -E "^import|Plugin" packages/vite/src/index.ts | head -20Length of output: 559
🏁 Script executed:
#!/bin/bash
# Get lines 1-50 carefully
head -50 packages/vite/src/index.tsLength of output: 1394
🏁 Script executed:
#!/bin/bash
# Now get the middle section with Plugin definition
sed -n '25,50p' packages/vite/src/index.tsLength of output: 721
🏁 Script executed:
#!/bin/bash
# Check what gets imported from vite
sed -n '1,20p' packages/vite/src/index.tsLength of output: 623
🏁 Script executed:
#!/bin/bash
# Check all files in packages/vite directory
find packages/vite -type f \( -name "*.ts" -o -name "*.tsx" \) | head -20Length of output: 171
🏁 Script executed:
#!/bin/bash
# Look for both babel and oxc files
find packages/vite -name "babel*" -o -name "oxc*" -o -name "*.ts"Length of output: 171
🏁 Script executed:
#!/bin/bash
# Check for index.ts location and structure
stat packages/vite/src/index.ts 2>&1 || echo "File not found, searching..."
find packages -name "index.ts" -path "*/vite/*"Length of output: 98
🏁 Script executed:
#!/bin/bash
# Look at the vite.config.ts to understand context better
sed -n '1,20p' examples/react-oxc/vite.config.tsLength of output: 312
🏁 Script executed:
#!/bin/bash
# Get lines around line 9
sed -n '5,15p' examples/react-oxc/vite.config.tsLength of output: 143
🏁 Script executed:
#!/bin/bash
# Show full vite config
cat examples/react-oxc/vite.config.ts 2>/dev/null || ls -la examples/Length of output: 312
🏁 Script executed:
#!/bin/bash
# Check exact location of files
find . -path "./packages/vite/src/index.ts" -o -path "./examples/react-oxc/vite.config.ts" 2>/dev/nullLength of output: 64
🏁 Script executed:
#!/bin/bash
# List directory structure
ls -la packages/ examples/ 2>/dev/null | head -40Length of output: 1010
🏁 Script executed:
#!/bin/bash
# Direct check
test -f packages/vite/src/index.ts && echo "File exists" || find . -name "index.ts" -path "*vite*" 2>/dev/nullLength of output: 11
🏁 Script executed:
#!/bin/bash
# Print complete vite config file
cat "examples/react-oxc/vite.config.ts"Length of output: 312
🏁 Script executed:
#!/bin/bash
# And the vite plugin file
cat "packages/vite/src/index.ts"Length of output: 1394
🏁 Script executed:
#!/bin/bash
# Check for PluginOption type definition in Vite
python3 << 'EOFY'
# Vite defines PluginOption as a union type that includes Plugin | null | false
# Check the Vite documentation
import subprocess
result = subprocess.run(['rg', 'PluginOption.*=', 'packages/vite/src/', '--type', 'ts'], capture_output=True, text=True)
print("PluginOption definitions found:")
print(result.stdout)
if not result.stdout:
print("No PluginOption definitions found in mincho packages")
EOFYLength of output: 85
🏁 Script executed:
#!/bin/bash
# Get comprehensive view - directly read and show relevant content
echo "=== vite.config.ts ===" && cat examples/react-oxc/vite.config.ts && echo -e "\n=== packages/vite/src/index.ts ===" && cat packages/vite/src/index.tsLength of output: 1766
🏁 Script executed:
#!/bin/bash
# Try a different approach - web search for Vite Plugin vs PluginOption
# but first check what we have
echo "Checking repo structure..."
ls -la packages/vite/src/ 2>/dev/null || echo "Directory not found"
echo "---"
ls -la examples/react-oxc/ 2>/dev/null | grep vite || echo "Files not found"Length of output: 504
🏁 Script executed:
#!/bin/bash
# Absolute final attempt - direct file read with python
python3 -c "
import os
files = [
'packages/vite/src/index.ts',
'examples/react-oxc/vite.config.ts'
]
for f in files:
if os.path.exists(f):
print(f'=== {f} ===')
with open(f) as fp:
print(fp.read())
print()
else:
print(f'NOT FOUND: {f}')
"Length of output: 1788
🏁 Script executed:
#!/bin/bash
# Check working directory and list structure
pwd && echo "---" && find . -maxdepth 3 -type f -name "*.ts" | grep -E "(vite|index)" | head -20Length of output: 508
🏁 Script executed:
#!/bin/bash
# Verify file locations with absolute paths
for path in "packages/vite/src/index.ts" "examples/react-oxc/vite.config.ts"; do
if [ -f "$path" ]; then
echo "✓ Found: $path"
wc -l < "$path"
else
echo "✗ Not found: $path"
fi
doneLength of output: 84
🏁 Script executed:
#!/bin/bash
# Use find to locate the files
echo "Looking for vite plugin files..."
find . -type f -name "*.ts" | xargs grep -l "minchoVitePlugin" 2>/dev/nullLength of output: 135
🏁 Script executed:
#!/bin/bash
# Get any .ts file that exports minchoVitePlugin
rg "minchoVitePlugin" --type ts -lLength of output: 96
🏁 Script executed:
#!/bin/bash
# Use the output from the original script - show context around the function
sed -n '20,50p' packages/vite/src/index.ts || echo "File structure unclear, searching for it..."
rg -n "export function minchoVitePlugin" --type ts -A30Length of output: 1934
🏁 Script executed:
#!/bin/bash
# Look at imports in the mincho file (around line 1-20)
rg -n "^import|^export" packages/vite/src/index.ts | head -30Length of output: 318
🏁 Script executed:
#!/bin/bash
# Search for Plugin type - where is it imported from?
rg -n "Plugin" packages/vite/src/index.ts | head -15Length of output: 468
🏁 Script executed:
#!/bin/bash
# Check the original output - it showed packages/vite/src/index.ts exists
# Let's get its imports
head -10 packages/vite/src/index.tsLength of output: 322
🏁 Script executed:
#!/bin/bash
# Get lines 1-10 from the vite package file
sed -n '1,10p' packages/vite/src/index.tsLength of output: 322
🌐 Web query:
Vite PluginOption type definition union
💡 Result:
Short answer: PluginOption is a union that accepts a single Plugin, an array of Plugins, or falsy nullable values:
PluginOption = Plugin | Plugin[] | false | null | undefined.
(Common TypeScript errors like "PluginOption[] is not assignable to PluginOption" typically come from mismatched / duplicated vite versions in node_modules causing incompatible type identities.) [1][2]
🌐 Web query:
Vite Plugin vs PluginOption types
💡 Result:
Short answer
- Plugin (import { Plugin } from 'vite') is the specific plugin object/interface (the plugin instance type).
- PluginOption is the config-level union/type accepted in vite's plugins field (it accepts Plugin, Plugin[], false/null, etc., and is what plugin factories often return or what vite.config.plugins expects). (phillipring.com)
Why you see type errors
- Type errors like "Plugin is not assignable to PluginOption" commonly come from multiple Vite versions in node_modules or mismatched type origins (the Plugin type from one Vite copy ≠ the Plugin type from another). That makes otherwise identical types incompatible. (github.com)
Practical fixes
- Ensure a single Vite version (npm/yarn/pnpm dedupe, remove duplicate vite deps). (github.com)
- If a plugin factory returns Plugin[] but a consumer expects PluginOption, use the correct export type (some plugins now return Plugin[] instead of PluginOption[]), or cast/normalize: e.g. plugins: [ ... ] as PluginOption or wrap/flatten arrays to match the expected shape. (phillipring.com)
If you want, tell me your package.json or npm ls vite output and I’ll point out duplicate vite installs and the exact fix.
Citations:
- 1: https://phillipring.com/vitejs/vite-plugin-react/releases?utm_source=openai
- 2: Vite 6 + React Router 7 - Type PluginOption[] is not assignable to type PluginOption vitest-dev/vitest#7075
- 3: Vite 6 + React Router 7 - Type PluginOption[] is not assignable to type PluginOption vitest-dev/vitest#7075
- 4: https://phillipring.com/vitejs/vite-plugin-react/releases?utm_source=openai
🌐 Web query:
Vite plugin array type compatibility
💡 Result:
Short answer: this TypeScript error usually means multiple incompatible copies of Vite/Rollup types are present (so a Plugin from one copy doesn't match PluginOption from another). Fixes: ensure a single vite/rollup version (dedupe / remove duplicates), use package manager overrides/resolutions, or cast as a compatible type as a last-resort workaround. Examples and guidance below.
What’s happening
- Vite’s plugins API accepts PluginOption which can be Plugin, Plugin[], or presets, but TypeScript will error if the Plugin type comes from a different copy of Vite/Rollup than the PluginOption you’re assigning to. This commonly happens after mixed/duplicate dependencies (multiple vite installs or mismatched rollup versions). [1][2]
How to fix (try in this order)
- Remove duplicates:
- delete node_modules and lockfile, reinstall (npm/yarn/pnpm).
- run npm dedupe / pnpm install --shamefully-hoist or pnpm dedupe. This often resolves the type mismatch. [2][3]
- Force one version:
- Use package manager overrides/resolutions to pin a single vite (and rollup) version for the whole tree (npm/PNPM overrides, yarn resolutions). Then reinstall. [2][3]
- Align peer deps:
- Ensure plugins that bundle their own vite/rollup types don’t introduce a second vite; prefer plugin versions that declare vite/rollup as peerDeps. [2]
- Short-term/workaround (if you must):
- Cast the plugin array to the expected type: plugins: [ ... ] as unknown as PluginOption (or import PluginOption from 'vite' and assert). Use only until you’ve resolved duplicate deps. This silences TS but doesn’t fix the root cause. [3][4]
References
- Vite plugins docs (plugins array/preset behavior). [1]
- GitHub issues showing the “PluginOption / Plugin[] type is not assignable” root cause and duped vite copies, and common fixes (dedupe/overrides). [2]
- Community threads (Vitest/StackOverflow) with the same TS error and suggested resolutions (delete node_modules, dedupe, pin/resolve versions, or cast). [3][4]
Would you like me to inspect your package.json / npm ls output and suggest exact overrides/commands for your project?
Remove the double type cast and align types properly.
The as unknown as PluginOption cast masks a type incompatibility. The mincho plugin exports a custom Plugin interface from packages/vite/src/types.ts rather than using Vite's Plugin type. While structurally compatible, they are nominally different types, which is why the double assertion is needed.
To fix:
- Export
PluginOptionfrom the mincho vite package by importing and re-exporting Vite's types, or updatepackages/vite/src/types.tsto extend/reference Vite's types directly rather than defining a customPlugininterface. - Declare
viteas a peerDependency inpackages/vite/package.jsonto ensure type alignment across the package boundary. - Remove the cast from
examples/react-oxc/vite.config.tsline 9 once the type is properly aligned.
This ensures type safety and makes the plugin's contract with Vite explicit.
🤖 Prompt for AI Agents
In examples/react-oxc/vite.config.ts around line 9, the use of "as unknown as
PluginOption" masks a real type mismatch between the mincho plugin’s custom
Plugin type and Vite’s PluginOption; fix by updating the mincho vite package so
its types align with Vite: modify packages/vite/src/types.ts to import and
re-export Vite’s Plugin/PluginOption (or make the custom interface
extend/reference Vite’s Plugin type) and ensure the package’s public typings
export that PluginOption, add "vite" as a peerDependency in
packages/vite/package.json so consumers share the same Vite types, then remove
the double type assertion from examples/react-oxc/vite.config.ts once the
package-level types match.
packages/oxc/eslint.config.js
Outdated
| import eslintConfigBase from "eslint-config-custom/eslint.config.base.js"; | ||
| import eslintConfigTypescript from "eslint-config-custom/eslint.config.typescript.js"; | ||
|
|
||
| export default [...eslintConfigBase, ...eslintConfigTypescript]; |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Verify that imports return config arrays.
The ESLint flat config composition pattern is correct. The spread syntax properly merges base and TypeScript-specific rules.
Verify that both imported modules export config arrays (not functions):
🏁 Script executed:
#!/bin/bash
# Description: Verify the ESLint config imports return arrays, not functions.
# Check the exports from both config files
echo "=== Checking eslint.config.base.js ==="
cat packages/configs/eslint-config-custom/eslint.config.base.js | tail -20
echo -e "\n=== Checking eslint.config.typescript.js ==="
cat packages/configs/eslint-config-custom/eslint.config.typescript.js | tail -20Length of output: 274
🏁 Script executed:
#!/bin/bash
# Find eslint config files in the repository
echo "=== Searching for eslint config files ==="
fd -e "js" "eslint" | head -20
echo -e "\n=== Searching for eslint-config-custom ==="
fd "eslint-config-custom"
echo -e "\n=== Listing packages directory ==="
fd --type d "packages" | head -10Length of output: 848
🏁 Script executed:
#!/bin/bash
# Search for where eslint configs are exported
echo "=== Searching for eslintConfig exports ==="
rg -i "export.*eslintConfig" --type js -A 2 -B 2
echo -e "\n=== Searching for eslint-config references ==="
rg "eslint-config" --type js | head -20Length of output: 4050
🏁 Script executed:
#!/bin/bash
# Check the actual exports from the base and typescript config files
echo "=== Checking eslint.config.base.js export ==="
cat configs/eslint-config-custom/eslint.config.base.js | grep -A 5 "^export"
echo -e "\n=== Checking eslint.config.typescript.js export ==="
cat configs/eslint-config-custom/eslint.config.typescript.js | grep -A 5 "^export"
echo -e "\n=== Full view of eslint.config.base.js end ==="
tail -10 configs/eslint-config-custom/eslint.config.base.js
echo -e "\n=== Full view of eslint.config.typescript.js end ==="
tail -10 configs/eslint-config-custom/eslint.config.typescript.jsLength of output: 925
🏁 Script executed:
#!/bin/bash
# Check if there's a default export at the end of the typescript config file
echo "=== Full eslint.config.typescript.js file ==="
cat configs/eslint-config-custom/eslint.config.typescript.js
echo -e "\n=== Checking for export default in typescript file ==="
grep "export default" configs/eslint-config-custom/eslint.config.typescript.js || echo "No default export found"Length of output: 2105
Fix import statement: cannot import named export as default.
The file configs/eslint-config-custom/eslint.config.typescript.js exports a named function eslintConfig, not a default export. The import in packages/oxc/eslint.config.js attempts to import it as a default, which will result in undefined and cause a runtime error when spreading.
Update the import and call the function:
import eslintConfigBase from "eslint-config-custom/eslint.config.base.js";
import { eslintConfig } from "eslint-config-custom/eslint.config.typescript.js";
export default [...eslintConfigBase, ...eslintConfig()];🤖 Prompt for AI Agents
In packages/oxc/eslint.config.js around lines 1 to 4 the file incorrectly
imports the TypeScript config as a default export which is actually a named
export, causing undefined when spreading; change the second import to import the
named export (eslintConfig) from the typescript config and call it (i.e., spread
eslintConfig() along with eslintConfigBase) so the exported array uses the
function's returned config rather than an undefined value.
| try { | ||
| // Parse to validate syntax | ||
| void parseSync(filename, code, { | ||
| sourceType: "module", | ||
| }); | ||
|
|
||
| // Check if there's a styled import | ||
| hasStyledImport = | ||
| /import\s+\{[^}]*\bstyled\b[^}]*\}\s+from\s+["']@mincho-js\/react["']/.test( | ||
| code | ||
| ); | ||
|
|
||
| if (!hasStyledImport) { | ||
| return { | ||
| code, | ||
| map: null, | ||
| cssExtractions, | ||
| dependencies, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Risk: Removing styled import even when no calls are successfully transformed
Flow today:
- Line 37–49: you detect any
styledimport and, if none, return early. - Lines 51–57: you always remove the
styledimport from@mincho-js/react. - Lines 60–122: you attempt to transform
styled(...)calls, but only when they match a specific shape:- There must be a comma (Line 65).
- The second argument must be parsed as an object literal by
findMatchingBrace(Line 70–73).
If a call doesn’t meet these assumptions (e.g. styled(Button, someStylesVar) or any more complex pattern), it is left untouched, yet the original styled import has already been removed. That yields a runtime ReferenceError: styled is not defined for those usages.
At minimum, this should be guarded so the import is only removed when at least one call is actually transformed to $$styled, or the transform should bail out entirely (leaving code as-is) when no safe transformations can be applied.
One concrete direction:
- Track whether any call was successfully transformed (e.g.
let transformedCount = 0;). - Only remove the
styledimport and add$$styled/rulesimports iftransformedCount > 0. - If
styledis imported but no supported call shape is found, return the original code untouched.
This will avoid silently breaking user code that uses styled in unsupported patterns.
Also applies to: 51-57, 60-122
🤖 Prompt for AI Agents
In packages/oxc/src/mincho-transform.ts around lines 30-49 (also affecting 51-57
and 60-122): the transform currently detects any styled import and
removes/rewrites imports even when no styled(...) calls are safely transformed,
which can break code using unsupported patterns. Fix by tracking whether any
call was actually transformed (e.g. transformedCount), only perform removal of
the original styled import and insertion of $$styled/rules imports when
transformedCount > 0, and if styled is present but no supported call shapes are
found, bail out and return the original code untouched so unsupported usages
keep working.
| * Check if a comment contains "mincho-ignore" | ||
| * @param comment - Comment text | ||
| * @returns True if comment contains mincho-ignore | ||
| */ | ||
| export function isIgnoreComment(comment: string): boolean { | ||
| return comment.trim() === "mincho-ignore" || comment.trim() === "mincho-js-ignore"; | ||
| } |
There was a problem hiding this comment.
isIgnoreComment docstring says “contains” but implementation requires exact match
Right now only comments whose trimmed content is exactly "mincho-ignore" or "mincho-js-ignore" are treated as ignore markers; anything like // mincho-ignore because ... will not be honored. That’s stricter than “contains” in the docstring and may surprise users.
Consider aligning behavior with the wording, e.g.:
-export function isIgnoreComment(comment: string): boolean {
- return comment.trim() === "mincho-ignore" || comment.trim() === "mincho-js-ignore";
-}
+export function isIgnoreComment(comment: string): boolean {
+ const text = comment.trim();
+ return (
+ text.includes("mincho-ignore") ||
+ text.includes("mincho-js-ignore")
+ );
+}(or a regex if you want stricter word-boundary matching).
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit fb3547bc9ce238bb2d7768ab44d3ad5e95534ecb (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Sun Dec 7 21:50:23 2025 +0900
Feat: Add new example and package for OXC integration with Mincho.jsIt is possible to fast forward |
…nd example references
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit 26dda39c3bd0a559f8c172a0c4b431428a981f2e (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Sun Dec 7 22:02:19 2025 +0900
Chore: Update TypeScript configuration files to include new package and example referencesIt is possible to fast forward |
…ss all configurations and examples
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit 601c254fab22a17ab0ac65e6ea819dad29881cba (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Mon Dec 8 00:30:24 2025 +0900
Chore: Upgrade ESLint and related dependencies to version 9.39.0 across all configurations and examplesIt is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
configs/vite-config-custom/tsconfig.node.json (1)
4-7: Configuration changes support declaration-focused builds with composite project references.The addition of
composite,declaration, andemitDeclarationOnlyoptions is appropriate for the build setup. However, line 5 (noEmit: false) is redundant:emitDeclarationOnly: truealready implies that emission is enabled (declarations only). Removing this line would improve clarity without changing behavior."compilerOptions": { "composite": true, - "noEmit": false, "declaration": true, "emitDeclarationOnly": true, "tsBuildInfoFile": "./.cache/typescript/tsbuildinfo-node", "outDir": "./.cache/tsbuild-node" },examples/react-oxc/package.json (2)
9-9: Consider aligning lint script with other examples.The lint script here is minimal (
eslint .) while other example packages use additional flags like--cache --cache-location .cache/eslint --report-unused-disable-directives --max-warnings 0. Consider aligning for consistency:- "lint": "eslint .", + "lint": "eslint . --cache --cache-location .cache/eslint --report-unused-disable-directives --max-warnings 0",
18-19: React version differs from other examples.This package uses React
^19.1.1whilereact-babelandreact-swcuse^19.2.0. Consider aligning versions for consistency across examples.configs/eslint-config-custom/eslint.config.typescript.js (2)
10-10: Unused typedef.
FlatConfigis defined but not used in this file. Consider removing it or documenting its intended use for consumers importing this module.
19-21: Redundant nullish coalescing.The
?? {}on line 21 is unnecessary sinceuserConfigsalready defaults to[]in the function signature. Additionally, whenuserConfigsis not an array, the value would never be nullish at that point.const normalizedUserConfigs = Array.isArray(userConfigs) ? userConfigs - : [userConfigs ?? {}]; + : [userConfigs];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
configs/eslint-config-custom/eslint.config.typescript.js(2 hunks)configs/eslint-config-custom/package.json(1 hunks)configs/tsconfig-custom/tsconfig.base.json(1 hunks)configs/vite-config-custom/tsconfig.json(1 hunks)configs/vite-config-custom/tsconfig.node.json(1 hunks)examples/react-babel/package.json(1 hunks)examples/react-oxc/package.json(1 hunks)examples/react-swc/eslint.config.js(1 hunks)examples/react-swc/package.json(2 hunks)package.json(1 hunks)packages/babel/tsconfig.lib.json(1 hunks)packages/oxc/tsconfig.node.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/oxc/tsconfig.node.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-03T11:16:32.528Z
Learnt from: black7375
Repo: mincho-js/mincho PR: 134
File: packages/transform-to-vanilla/src/transform.ts:49-51
Timestamp: 2024-12-03T11:16:32.528Z
Learning: In projects that target both CommonJS and ES modules, adjusting the TypeScript `module` option may not be feasible. Therefore, using `ts-ignore` comments to suppress TypeScript errors related to `import.meta` is acceptable.
Applied to files:
configs/tsconfig-custom/tsconfig.base.json
🧬 Code graph analysis (1)
configs/vite-config-custom/tsconfig.json (1)
configs/vite-config-custom/src/index.ts (2)
NodeConfig(24-29)NodeBuilder(45-128)
🔇 Additional comments (9)
configs/vite-config-custom/tsconfig.json (1)
3-5: Composite build setup aligns with monorepo integration.Adding
composite: truesupports the project reference structure already defined in the file and is appropriate for a monorepo setup with OXC integration.examples/react-babel/package.json (1)
38-38: LGTM!ESLint version bumps to 9.39.0 with exact pinning align with the coordinated update across the monorepo.
Also applies to: 46-46
examples/react-swc/package.json (1)
18-18: LGTM!ESLint dependency updates are consistent with the monorepo-wide version alignment.
Also applies to: 28-28
package.json (1)
61-61: LGTM!Root-level ESLint pinned to exact version ensures consistent tooling across the monorepo.
configs/eslint-config-custom/package.json (1)
21-21: LGTM!ESLint version updates align with the monorepo-wide upgrade.
Also applies to: 24-24
examples/react-swc/eslint.config.js (1)
38-40: LGTM!The removal of manual
react-hooksplugin registration from thepluginsobject is correct sincereactHooksPlugin.configs.flat.recommended(line 16) already registers the plugin through the flat config API.configs/eslint-config-custom/eslint.config.typescript.js (1)
16-16: LGTM!The signature update to accept
TSConfig | TSConfig[]improves API flexibility, and switching totseslint.config()is the correct approach for TypeScript-ESLint configuration composition.Also applies to: 23-23, 66-66
examples/react-oxc/package.json (1)
12-16: No action needed.@mincho-js/oxcis correctly accessed as a transitive dependency through@mincho-js/vite, which includes it as a direct dependency. The example only needs to list@mincho-js/viteas a direct dependency, and OXC functionality is automatically available via the unifiedminchoVitePlugin()which defaults to the OXC implementation.packages/babel/tsconfig.lib.json (1)
7-10: Path alias and declaration directory updates align with monorepo standardization.The additions of
declarationDirand the package-scoped@/*paths mapping are appropriate and follow the monorepo's move toward consistent path aliasing withoutbaseUrl. The package-level paths correctly override the inherited base config paths, ensuring this package's imports resolve to./src/*relative topackages/babel/.Confirm that all code within this package and its dependents properly use or don't rely on the
@/*alias in ways that might break with this configuration.
| }, | ||
| "devDependencies": { | ||
| "@eslint/js": "^9.38.0", | ||
| "@eslint/core": "0.17.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "@eslint/core" configs/eslint-config-custom/Repository: mincho-js/mincho
Length of output: 42
🏁 Script executed:
# Check if @eslint/core is mentioned in package.json or if it's a peer dependency
cat configs/eslint-config-custom/package.json | grep -A 50 "dependencies\|peerDependencies"Repository: mincho-js/mincho
Length of output: 42
🏁 Script executed:
# Also check if there are any comments or documentation about this dependency
rg -i "eslint.*core|core.*eslint" configs/eslint-config-custom/ -B 2 -A 2Repository: mincho-js/mincho
Length of output: 42
🏁 Script executed:
cat configs/eslint-config-custom/package.jsonRepository: mincho-js/mincho
Length of output: 933
🏁 Script executed:
find configs/eslint-config-custom -name "*.js" -type fRepository: mincho-js/mincho
Length of output: 218
🏁 Script executed:
cat configs/eslint-config-custom/eslint.config.typescript.jsRepository: mincho-js/mincho
Length of output: 2053
🏁 Script executed:
cat configs/eslint-config-custom/eslint.config.base.js && echo "---" && cat configs/eslint-config-custom/eslint.config.yaml.jsRepository: mincho-js/mincho
Length of output: 1379
Remove unused @eslint/core dependency.
The @eslint/core package is added to devDependencies but is not imported or used anywhere in the package. The ESLint configuration files import from @eslint/js, eslint, and typescript-eslint, but never reference @eslint/core. This dependency should be removed.
🤖 Prompt for AI Agents
In configs/eslint-config-custom/package.json around line 20, remove the unused
devDependency entry for "@eslint/core": "0.17.0" because the package is not
imported or referenced anywhere in the repo; update the package.json by deleting
that dependency line and run npm/yarn install (or npm prune) to update lockfile
and node_modules accordingly.
| "paths": { /* Specify a set of entries that re-map imports to additional lookup locations. */ | ||
| "@/*": ["src/*"] | ||
| "@/*": ["./src/*"] | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the base config and understand the structure
find . -name "tsconfig.base.json" -o -name "tsconfig*.json" | head -30Repository: mincho-js/mincho
Length of output: 1256
🏁 Script executed:
# Get monorepo structure
ls -la | grep -E "^d"Repository: mincho-js/mincho
Length of output: 707
🏁 Script executed:
# Find all tsconfig files extending the base config
rg -l "tsconfig-custom/tsconfig.base.json" --type jsonRepository: mincho-js/mincho
Length of output: 505
🏁 Script executed:
# For each file that extends the base config, check if it overrides paths
rg "tsconfig-custom/tsconfig.base.json" --type json -A 50 | grep -E "(tsconfig|paths|@/)"Repository: mincho-js/mincho
Length of output: 12186
🏁 Script executed:
# Check which packages actually use @/* imports
rg "@/" --type ts --type tsx | head -50Repository: mincho-js/mincho
Length of output: 87
🏁 Script executed:
# Specifically check packages/oxc which extends base without overriding paths or baseUrl
cat packages/oxc/tsconfig.jsonRepository: mincho-js/mincho
Length of output: 456
🏁 Script executed:
# Check if packages that extend base actually have src directories at the expected locations
find packages -type d -name "src" | head -20Repository: mincho-js/mincho
Length of output: 286
🏁 Script executed:
# Look at the base config more closely to understand paths definition
cat configs/tsconfig-custom/tsconfig.base.jsonRepository: mincho-js/mincho
Length of output: 13123
🏁 Script executed:
# Check for @/ imports in packages/oxc
rg "@/" packages/oxc/ --type tsRepository: mincho-js/mincho
Length of output: 42
🏁 Script executed:
# Check all packages for @/ usage to understand scope of issue
rg "@/" --type ts -c | sort -rnRepository: mincho-js/mincho
Length of output: 643
🏁 Script executed:
# Look at packages/oxc/src to see what it imports
ls -la packages/oxc/src/Repository: mincho-js/mincho
Length of output: 601
🏁 Script executed:
# Check if there are any import errors or build warnings related to path resolution
rg "Cannot find" --type ts --type js 2>/dev/null || echo "No errors found"Repository: mincho-js/mincho
Length of output: 75
🏁 Script executed:
# Double-check packages that extend base without baseUrl override
# to ensure they don't use @/ imports
rg "@/" packages/debug-log/ --type ts
rg "@/" packages/css-additional-types/ --type ts
rg "@/" configs/vite-config-custom/ --type tsRepository: mincho-js/mincho
Length of output: 42
🏁 Script executed:
# Verify the actual path resolution by checking packages that use @/
# Look at transform-to-vanilla which uses @/ and sets baseUrl
cat packages/transform-to-vanilla/tsconfig.lib.json | grep -A 5 "baseUrl\|paths"Repository: mincho-js/mincho
Length of output: 225
Base config's path mappings work in practice, but the design lacks defensiveness.
The change from "src/*" to "./src/*" is correct and explicit. Without baseUrl in the base config, these paths resolve relative to configs/tsconfig-custom/, which doesn't contain src/. However, all packages currently using @/* imports either override baseUrl: "./" (transform-to-vanilla, integration) or explicitly override paths (babel), so this works correctly in practice.
The design risk remains valid: if a future package uses @/* imports without setting baseUrl, it will silently fail to resolve. Packages not using the alias (oxc, debug-log, css-additional-types) safely inherit the broken paths without consequence.
Consider documenting that all packages extending this config must either set baseUrl or explicitly override paths, or alternatively, remove paths from the base config entirely and require all packages to define it.
🤖 Prompt for AI Agents
In configs/tsconfig-custom/tsconfig.base.json around lines 32-34, the shared
"paths" mapping uses "./src/*" but the base config lacks a baseUrl so those
paths resolve relative to the configs folder and can silently break for packages
that don't override baseUrl; update the base config to be defensive by either
(A) add "baseUrl": "./" alongside "paths" so the alias reliably resolves to the
repository root for inheriting projects, or (B) remove "paths" from this base
file and require each package tsconfig to define its own "baseUrl" and "paths";
alternatively add a short comment/docblock in the base file instructing package
authors that any config extending this must explicitly set baseUrl or override
paths.
| "@vitejs/plugin-react": "^5.0.4", | ||
| "babel-plugin-react-compiler": "^19.1.0-rc.3", | ||
| "eslint": "9.39.0", | ||
| "eslint-plugin-react-hooks": "^5.2.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check eslint-plugin-react-hooks versions across examples
rg -n "eslint-plugin-react-hooks" examples/*/package.jsonRepository: mincho-js/mincho
Length of output: 295
Consolidate eslint-plugin-react-hooks versions across examples.
react-oxc uses ^5.2.0 while react-babel and react-swc both use ^7.0.0. This major version difference can cause inconsistent linting behavior across the examples and should be aligned.
🤖 Prompt for AI Agents
In examples/react-oxc/package.json around line 29, the project pins
"eslint-plugin-react-hooks" to "^5.2.0" which is a major-version mismatch with
the "^7.0.0" used in react-babel and react-swc; update the dependency to the
same major version (e.g., "^7.0.0") to align linting across examples, run
npm/yarn install to update lockfile, and verify linting passes in the react-oxc
example.
…nitions and output directory for OXC
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit 06b5c25ab483f6d808e2c0b9d8f40385a8edbcd6 (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Mon Dec 8 01:12:29 2025 +0900
Chore: Update package.json and tsconfig.lib.json to include type definitions and output directory for OXCIt is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/oxc/package.json(1 hunks)packages/oxc/tsconfig.lib.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/oxc/tsconfig.lib.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: typecheck
🔇 Additional comments (1)
packages/oxc/package.json (1)
69-69: Theprettier-config-custompackage exists atconfigs/prettier-config-custom/with a validpackage.jsondeclaring"name": "prettier-config-custom". The workspace root configuration includes"configs/*", which properly links this package. The reference in line 69 ofpackages/oxc/package.jsonis correctly resolved.
| "exports": { | ||
| ".": { | ||
| "import": { | ||
| "types": "./dist/esm/index.d.ts", | ||
| "default": "./dist/esm/index.mjs" | ||
| }, | ||
| "require": { | ||
| "types": "./dist/cjs/index.d.cts", | ||
| "default": "./dist/cjs/index.cjs" | ||
| } | ||
| }, | ||
| "./mincho-transform": { | ||
| "import": { | ||
| "types": "./dist/esm/mincho-transform.d.ts", | ||
| "default": "./dist/esm/mincho-transform.mjs" | ||
| }, | ||
| "require": { | ||
| "types": "./dist/cjs/mincho-transform.d.cts", | ||
| "default": "./dist/cjs/mincho-transform.cjs" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find build configuration files
echo "=== Looking for build config files ==="
fd -type f \( -name 'tsconfig*' -o -name 'vite.config*' -o -name 'rollup.config*' -o -name 'webpack.config*' -o -name 'build.ts' -o -name 'build.js' \) packages/oxc/
echo "=== Checking packages/oxc directory structure ==="
ls -la packages/oxc/
echo "=== Checking if dist directory exists ==="
if [ -d "packages/oxc/dist" ]; then
echo "dist directory found. Listing contents:"
find packages/oxc/dist -type f | sort
else
echo "dist directory does not exist"
fi
echo "=== Checking package.json for build scripts ==="
jq '.scripts' packages/oxc/package.json 2>/dev/null || grep -A 10 '"scripts"' packages/oxc/package.jsonRepository: mincho-js/mincho
Length of output: 1409
🏁 Script executed:
#!/bin/bash
echo "=== Reading vite.config.ts ==="
cat packages/oxc/vite.config.ts
echo ""
echo "=== Checking source structure ==="
find packages/oxc/src -type f | sort
echo ""
echo "=== Checking for root vite config ==="
if [ -f vite.config.ts ]; then
head -50 vite.config.ts
fi
echo ""
echo "=== Reading package.json build related config ==="
cat packages/oxc/package.jsonRepository: mincho-js/mincho
Length of output: 3008
🏁 Script executed:
#!/bin/bash
echo "=== Looking for vite-config-custom ==="
find . -path ./node_modules -prune -o -name 'vite-config-custom*' -type d -print
echo ""
echo "=== Checking for vite-config-custom in workspace packages ==="
ls -la packages/ | grep -i vite
echo ""
echo "=== Finding vite-config-custom source ==="
fd 'vite-config-custom' --type d --max-depth 3
echo ""
echo "=== Checking vite-config-custom package.json ==="
if [ -d "packages/vite-config-custom" ]; then
cat packages/vite-config-custom/package.json
fi
echo ""
echo "=== Checking vite-config-custom source files ==="
if [ -d "packages/vite-config-custom" ]; then
find packages/vite-config-custom/src -type f 2>/dev/null | head -20
fiRepository: mincho-js/mincho
Length of output: 415
🏁 Script executed:
#!/bin/bash
echo "=== Listing configs/vite-config-custom ==="
ls -la configs/vite-config-custom/
echo ""
echo "=== Checking for main config file ==="
cat configs/vite-config-custom/package.json 2>/dev/null | head -30
echo ""
echo "=== Finding source files ==="
find configs/vite-config-custom -type f \( -name '*.ts' -o -name '*.js' \) | head -20Repository: mincho-js/mincho
Length of output: 1723
🏁 Script executed:
#!/bin/bash
echo "=== Reading vite-config-custom src/index.ts ==="
cat configs/vite-config-custom/src/index.tsRepository: mincho-js/mincho
Length of output: 4671
🏁 Script executed:
#!/bin/bash
echo "=== Checking for mincho-transform in vite config ==="
grep -r "mincho-transform" configs/vite-config-custom/
echo ""
echo "=== Checking git history for exports map ==="
cd packages/oxc
git log --oneline -20 -- package.json 2>/dev/null | head -10
echo ""
echo "=== Checking for any build-related files ==="
find . -name "*build*" -o -name "*entry*" | grep -v node_modules | grep -v dist
echo ""
echo "=== Looking for alternative entry point definitions ==="
rg "mincho-transform" . --type json --type ts 2>/dev/null | head -20Repository: mincho-js/mincho
Length of output: 949
Build configuration does not generate the ./mincho-transform export artifacts.
The exports map declares ./mincho-transform as a separate entry point with ESM and CJS variants, but the build configuration in configs/vite-config-custom/src/index.ts only defines index as an entry point (line 68-71). This means the build will generate index.mjs, index.cjs, index.d.ts, and index.d.cts, but will NOT generate the mincho-transform.mjs, mincho-transform.cjs, mincho-transform.d.ts, and mincho-transform.d.cts files declared in the exports map. Add mincho-transform: resolve(entryRoot, "mincho-transform.ts") to the entry object in the NodeBuilder function to generate separate artifacts for this export.
🤖 Prompt for AI Agents
In packages/oxc/package.json around lines 28 to 49, the package.json declares a
separate "./mincho-transform" export but the build config
(configs/vite-config-custom/src/index.ts lines ~68-71) only registers "index" as
an entry so the build won't emit mincho-transform.* artifacts; add an entry
mapping for mincho-transform in the NodeBuilder entry object by adding
mincho-transform: resolve(entryRoot, "mincho-transform.ts") so the bundler
generates mincho-transform.mjs/.cjs and corresponding .d.ts/.d.cts outputs to
match the exports map.
| "oxc-parser": "latest", | ||
| "oxc-transform": "latest", | ||
| "unplugin-oxc": "latest" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/oxc/package.json | sed -n '70,76p'Repository: mincho-js/mincho
Length of output: 283
🌐 Web query:
oxc-parser npm package latest stable version
💡 Result:
The latest stable version of the oxc-parser npm package is 0.86.0. [1]
[1] npm: oxc-parser package page.
🌐 Web query:
oxc-transform npm package latest stable version
💡 Result:
The latest stable oxc-transform release is v0.86.0. [1][2]
Sources:
[1] npm package listing for oxc-transform. (npmjs.com)
[2] Oxc project blog (transform announcement). (oxc.rs)
Citations:
- 1: https://www.npmjs.com/package/oxc-transform?utm_source=openai
- 2: https://oxc.rs/blog/2024-09-29-transformer-alpha?utm_source=openai
🌐 Web query:
unplugin-oxc npm package latest stable version
💡 Result:
The latest stable version of unplugin-oxc is 0.5.5 (npm/jsDelivr). [1]
(Note: some registry/analysis snapshots still list 0.4.6 — e.g., Socket's listing last updated Jun 26, 2025). [2]
Sources:
[1] jsDelivr package page for unplugin-oxc.
[2] Socket.dev package page for unplugin-oxc.
Pin OXC dependencies to specific versions.
Using "latest" for production dependencies (oxc-parser, oxc-transform, unplugin-oxc) creates non-deterministic builds and risks breaking changes from upstream. Replace with pinned semantic versions:
"dependencies": {
"@emotion/hash": "^0.9.2",
"magic-string": "^0.30.0",
- "oxc-parser": "latest",
- "oxc-transform": "latest",
- "unplugin-oxc": "latest"
+ "oxc-parser": "^0.86.0",
+ "oxc-transform": "^0.86.0",
+ "unplugin-oxc": "^0.5.5"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "oxc-parser": "latest", | |
| "oxc-transform": "latest", | |
| "unplugin-oxc": "latest" | |
| "dependencies": { | |
| "@emotion/hash": "^0.9.2", | |
| "magic-string": "^0.30.0", | |
| "oxc-parser": "^0.86.0", | |
| "oxc-transform": "^0.86.0", | |
| "unplugin-oxc": "^0.5.5" |
🤖 Prompt for AI Agents
In packages/oxc/package.json around lines 73 to 75, the three OXC dependencies
are set to "latest", which makes builds non-deterministic; replace their version
specifiers with pinned semantic versions (exact versions or caret ranges agreed
on by the team), e.g. change "oxc-parser": "latest", "oxc-transform": "latest",
"unplugin-oxc": "latest" to concrete version strings like "oxc-parser": "x.y.z",
"oxc-transform": "x.y.z", "unplugin-oxc": "x.y.z" (use the approved versions
from package-lock or your registry), then run the package manager restore to
update lockfile and ensure CI passes.
… new tsconfig.eslint.json and adjustments to type definitions
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit 40d180b444d58b7b0b8e86b3c9102cb33b0dbc8a (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Mon Dec 8 01:34:44 2025 +0900
Chore: Update TypeScript and ESLint configurations for OXC, including new tsconfig.eslint.json and adjustments to type definitionsIt is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
packages/oxc/src/utils.ts (1)
76-85:isIgnoreCommentdoesn’t match “contains” semantics and will miss inline ignore markersThe docstring says “contains
mincho-ignore”, but the implementation only matches comments that exactly equal"mincho-ignore"or"mincho-js-ignore"after trimming. This means cases like:/* mincho-ignore */ style({ color: "red" });or comments with an explanation will not be treated as ignore markers, which contradicts both the wording and the tests that use an inline
/* mincho-ignore */comment.Aligning behavior with “contains” semantics would also address the earlier review note on this function.
-export function isIgnoreComment(comment: string): boolean { - return ( - comment.trim() === "mincho-ignore" || comment.trim() === "mincho-js-ignore" - ); -} +export function isIgnoreComment(comment: string): boolean { + const text = comment.trim(); + return ( + text.includes("mincho-ignore") || + text.includes("mincho-js-ignore") + ); +}
🧹 Nitpick comments (1)
packages/oxc/src/utils.ts (1)
8-31: Consider makingextractionAPIsreadonly / literal-typedIf this array is meant to be a fixed set of recognized APIs, you might want to:
- Mark it
as constso downstream code can use a string-literal union type, and/or- Prevent accidental mutation at runtime.
Example:
-export const extractionAPIs = [ +export const extractionAPIs = [ // @mincho-js/css "mincho$", "css", "globalCss", "rules", // @vanilla-extract/css "style", "styleVariants", "globalStyle", "createTheme", "createGlobalTheme", "createThemeContract", "createGlobalThemeContract", "assignVars", "createVar", "fallbackVar", "fontFace", "globalFontFace", "keyframes", "globalKeyframes", // @vanilla-extract/recipes "recipe" -]; +] as const;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/css-additional-types/tsconfig.codegen.json(1 hunks)packages/oxc/eslint.config.js(1 hunks)packages/oxc/src/__tests__/base-transform.test.ts(1 hunks)packages/oxc/src/__tests__/mincho-transform.test.ts(1 hunks)packages/oxc/src/base-transform.ts(1 hunks)packages/oxc/src/global.d.ts(1 hunks)packages/oxc/src/index.ts(1 hunks)packages/oxc/src/mincho-transform.ts(1 hunks)packages/oxc/src/types.ts(1 hunks)packages/oxc/src/utils.ts(1 hunks)packages/oxc/tsconfig.eslint.json(1 hunks)packages/oxc/tsconfig.json(1 hunks)packages/oxc/vite.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/oxc/src/index.ts
- packages/oxc/src/tests/base-transform.test.ts
- packages/oxc/src/types.ts
- packages/oxc/vite.config.ts
- packages/oxc/tsconfig.json
- packages/oxc/src/base-transform.ts
- packages/oxc/eslint.config.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-03T11:16:32.528Z
Learnt from: black7375
Repo: mincho-js/mincho PR: 134
File: packages/transform-to-vanilla/src/transform.ts:49-51
Timestamp: 2024-12-03T11:16:32.528Z
Learning: In projects that target both CommonJS and ES modules, adjusting the TypeScript `module` option may not be feasible. Therefore, using `ts-ignore` comments to suppress TypeScript errors related to `import.meta` is acceptable.
Applied to files:
packages/css-additional-types/tsconfig.codegen.json
🧬 Code graph analysis (2)
packages/oxc/src/__tests__/mincho-transform.test.ts (2)
packages/oxc/src/index.ts (1)
minchoTransform(7-7)packages/oxc/src/mincho-transform.ts (1)
minchoTransform(15-169)
packages/oxc/tsconfig.eslint.json (1)
configs/eslint-config-custom/eslint.config.typescript.js (1)
eslintConfig(19-71)
🪛 ast-grep (0.40.0)
packages/oxc/src/mincho-transform.ts
[warning] 178-178: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b${functionName}\\s*\\(, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 GitHub Check: CodeQL
packages/oxc/src/mincho-transform.ts
[failure] 38-40: Polynomial regular expression used on uncontrolled data
This regular expression that depends on library input may run slow on strings starting with 'import {{' and with many repetitions of 'import {{'.
[failure] 55-55: Polynomial regular expression used on uncontrolled data
This regular expression that depends on library input may run slow on strings starting with 'import {{' and with many repetitions of 'import {{'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: typecheck
🔇 Additional comments (2)
packages/css-additional-types/tsconfig.codegen.json (1)
5-7: Verify the broad wildcard path mapping works as intended.The change replaces
baseUrl: "./"with a catch-all path pattern"*": ["./*"]. This broad wildcard will intercept all unresolved module resolutions, which differs from other tsconfig adjustments in this PR that use specific prefixes (e.g.,"@/*").Confirm that:
- The codegen phase still resolves all modules correctly (both Node/third-party packages and relative imports).
- This change doesn't cause unintended side effects in module resolution during code generation.
- The wildcard pattern is the intended behavior for this codegen context.
If the intent was to isolate the codegen to the current directory only, consider using a more explicit or documented pattern. Alternatively, clarify why this differs from the path mappings used in other tsconfig files (e.g.,
configs/tsconfig-custom/tsconfig.base.json).packages/oxc/src/global.d.ts (1)
5-71: Guard against type drift from the real OXC APIsThese ambient typings hard-code a narrow subset of options (e.g.
TransformOptions.target,ParseOptions,ParseError) that may not stay in sync with the actualoxc-transform/oxc-parserAPIs. If you intend to pass options through from upstream tools (e.g. unplugin-oxc) or future OXC versions, consider either:
- Re-exporting OXC’s official TypeScript types (if/when they’re published), or
- Relaxing these definitions (e.g. widening
targettostringor adding index signatures for unknown options) to avoid breaking consumers when OXC adds new flags.
| it("should extract style() calls to CSS modules", () => { | ||
| const code = ` | ||
| import { style } from "@mincho-js/css"; | ||
| const red = style({ color: "red" }); | ||
| `; | ||
|
|
||
| const result = minchoTransform(code, { | ||
| filename: "test.ts", | ||
| sourceRoot: process.cwd(), | ||
| extractCSS: true | ||
| }); | ||
|
|
||
| expect(result.cssExtractions.length).toBeGreaterThan(0); | ||
| expect(result.code).toContain("import"); | ||
| expect(result.code).toContain("extracted_"); | ||
| }); | ||
|
|
||
| it("should handle mincho-ignore comments", () => { | ||
| const code = ` | ||
| import { style } from "@mincho-js/css"; | ||
| const red = /* mincho-ignore */ style({ color: "red" }); | ||
| `; | ||
|
|
||
| const result = minchoTransform(code, { | ||
| filename: "test.ts", | ||
| sourceRoot: process.cwd(), | ||
| extractCSS: true | ||
| }); | ||
|
|
||
| // Should not extract when mincho-ignore is present | ||
| expect(result.cssExtractions.length).toBe(0); | ||
| }); |
There was a problem hiding this comment.
Tests expect style() extraction and mincho-ignore handling that the transform doesn’t implement yet
These two tests:
"should extract style() calls to CSS modules""should handle mincho-ignore comments"
are exercising behavior (style from @mincho-js/css and /* mincho-ignore */ comments) that the current minchoTransform implementation never touches—it only looks for styled imports and styled(...) calls.
Unless there’s another layer wrapping minchoTransform that you plan to call here, these tests will fail until you either:
- Extend
minchoTransform(or another OXC stage) to actually transformstyle()and honor ignore comments, and then call that API in the tests, or - Narrow the scope of this spec to
styled()-only behavior and movestyle()/ ignore coverage to the component that implements CSS extraction.
🤖 Prompt for AI Agents
In packages/oxc/src/__tests__/mincho-transform.test.ts around lines 31 to 62,
the tests exercise extraction of style() from "@mincho-js/css" and honoring /*
mincho-ignore */ comments, but minchoTransform currently only processes styled
imports/calls; either update the tests to target only the implemented styled()
behavior (replace style() cases with styled() or move style/ignore tests to the
module that implements extraction) or extend minchoTransform to also
detect/import "@mincho-js/css", transform style(...) calls into CSS-module
extractions, and skip extraction when a preceding /* mincho-ignore */ comment is
present; choose one approach and update tests or implementation accordingly so
tests and implementation match.
| hasStyledImport = | ||
| /import\s+\{[^}]*\bstyled\b[^}]*\}\s+from\s+["']@mincho-js\/react["']/.test( | ||
| code | ||
| ); | ||
|
|
||
| if (!hasStyledImport) { | ||
| return { | ||
| code, | ||
| map: null, | ||
| cssExtractions, | ||
| dependencies | ||
| }; | ||
| } | ||
|
|
||
| // Remove original styled import | ||
| const styledImportRegex = | ||
| /import\s+\{[^}]*\bstyled\b[^}]*\}\s+from\s+["']@mincho-js\/react["'];?\s*/g; | ||
| let importMatch; | ||
| while ((importMatch = styledImportRegex.exec(code)) !== null) { | ||
| s.remove(importMatch.index, importMatch.index + importMatch[0].length); | ||
| } |
There was a problem hiding this comment.
Risk: styled import is removed even if no calls are safely transformed
Flow today:
- Lines 37–42: any
styledimport from@mincho-js/reacttriggers the transform. - Lines 51–57: you always remove the
styledimport. - Lines 60–122: you only rewrite calls that match a specific
styled(tag, /* object literal */ …)shape.
If a file imports styled but uses it in an unsupported pattern (e.g. styled(Button, stylesVar), wrappers, or anything where findMatchingBrace/findFirstCommaOutsideParens fail), the import is removed but the call is left as-is, leading to ReferenceError: styled is not defined at runtime.
A safer pattern is:
- Scan for
styled(...)call sites and attempt rewrites. - Track how many calls were actually transformed.
- Only remove the original
styledimport and add$$styled/rulesimports if at least one call was rewritten; otherwise, return the original code untouched.
For example:
- // Remove original styled import
- const styledImportRegex =
- /import\s+\{[^}]*\bstyled\b[^}]*\}\s+from\s+["']@mincho-js\/react["'];?\s*/g;
- let importMatch;
- while ((importMatch = styledImportRegex.exec(code)) !== null) {
- s.remove(importMatch.index, importMatch.index + importMatch[0].length);
- }
-
- // Find all styled() calls
- const styledMatches = findFunctionCalls(code, "styled");
-
- for (const { start, end } of styledMatches) {
+ // Find all styled() calls
+ const styledMatches = findFunctionCalls(code, "styled");
+ let transformedCount = 0;
+
+ for (const { start, end } of styledMatches) {
const original = code.slice(start, end);
const argsStart = original.indexOf("(") + 1;
const firstComma = findFirstCommaOutsideParens(original, argsStart);
@@
- s.overwrite(start, end, transformed);
- imports.add("$$styled");
- imports.add(rulesVarName); // Track for import generation
+ s.overwrite(start, end, transformed);
+ imports.add("$$styled");
+ imports.add(rulesVarName); // Track for import generation
+ transformedCount++;
@@
- s.overwrite(start, end, transformed);
- imports.add("$$styled");
- imports.add("rules");
+ s.overwrite(start, end, transformed);
+ imports.add("$$styled");
+ imports.add("rules");
+ transformedCount++;
@@
- }
+ }
+
+ if (transformedCount === 0) {
+ // No supported `styled(...)` shapes; leave the file untouched.
+ return {
+ code,
+ map: null,
+ cssExtractions: [],
+ dependencies: []
+ };
+ }
+
+ // Now that we know we rewrote calls, remove the original styled import.
+ const styledImportRegex =
+ /import\s+\{[^}]*\bstyled\b[^}]*\}\s+from\s+["']@mincho-js\/react["'];?\s*/g;
+ let importMatch;
+ while ((importMatch = styledImportRegex.exec(code)) !== null) {
+ s.remove(importMatch.index, importMatch.index + importMatch[0].length);
+ }This also makes it easier to evolve the supported call shapes without risking silent breakage.
Separately, CodeQL has flagged the styled-import regex as potentially polynomial; if that becomes a blocker, you could take advantage of the existing parseSync call and inspect the AST imports instead of using a complex regex at all.
Also applies to: 60-122
🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 38-40: Polynomial regular expression used on uncontrolled data
This regular expression that depends on library input may run slow on strings starting with 'import {{' and with many repetitions of 'import {{'.
[failure] 55-55: Polynomial regular expression used on uncontrolled data
This regular expression that depends on library input may run slow on strings starting with 'import {{' and with many repetitions of 'import {{'.
| function findFunctionCalls( | ||
| code: string, | ||
| functionName: string | ||
| ): Array<{ start: number; end: number }> { | ||
| const results: Array<{ start: number; end: number }> = []; | ||
| const regex = new RegExp(`\\b${functionName}\\s*\\(`, "g"); | ||
| let match; | ||
|
|
||
| while ((match = regex.exec(code)) !== null) { | ||
| const start = match.index; | ||
| const openParenIndex = match.index + match[0].length - 1; | ||
| const endIndex = findMatchingClosingParen(code, openParenIndex); | ||
|
|
||
| if (endIndex !== -1) { | ||
| results.push({ start, end: endIndex + 1 }); | ||
| } | ||
| } | ||
|
|
||
| return results; | ||
| } |
There was a problem hiding this comment.
Dynamic regex in findFunctionCalls is safe today but still triggers ReDoS tooling warnings
findFunctionCalls builds a RegExp from functionName:
const regex = new RegExp(`\\b${functionName}\\s*\\(`, "g");Static analysis flags this as a potential ReDoS vector. In this file you only ever pass a hard-coded "styled", so it’s not exploitable in practice, but if this helper is reused with arbitrary input later, it could be.
Two low-effort hardening options:
- Sanitize/escape
functionNameand restrict it to identifier characters (e.g./^[A-Za-z_$][A-Za-z0-9_$]*$/), or - Ditch
RegExpentirely and do a simple loop usingindexOf(functionName + "(")plus a few identifier-boundary checks.
Given the current usage, this is mostly about silencing tools and future-proofing.
🧰 Tools
🪛 ast-grep (0.40.0)
[warning] 178-178: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b${functionName}\\s*\\(, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
In packages/oxc/src/mincho-transform.ts around lines 174 to 193, the dynamic
RegExp built from functionName can trigger ReDoS warnings; restrict or eliminate
arbitrary input by validating/escaping the functionName before using it or
replace the regex with a safe string scan. Implement one of these fixes: either
first assert functionName matches an identifier pattern
(/^[A-Za-z_$][A-Za-z0-9_$]*$/) and escape it before constructing the RegExp, or
avoid RegExp entirely and locate calls with a simple loop using
indexOf(functionName + "(") combined with boundary checks to ensure the match is
a standalone identifier; update the function to refuse or throw on invalid
functionName to prevent future misuse.
| { | ||
| "extends": "./tsconfig.json", | ||
| "compilerOptions": { | ||
| "allowJs": true, | ||
| "rootDir": ".", | ||
| "outDir": "./.cache/eslint" | ||
| }, | ||
| "include": [ | ||
| "eslint.config.js", | ||
| "src/**/*" | ||
| ], | ||
| "exclude": [ | ||
| "dist", | ||
| "node_modules" | ||
| ] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find packages/oxc -type f \( -name "eslint.config.js" -o -name ".eslintrc*" -o -name "eslint.config.*" \) 2>/dev/nullRepository: mincho-js/mincho
Length of output: 89
🏁 Script executed:
rg -A 15 "parserOptions|project" packages/oxc/ --type js --type json 2>/dev/null | head -50Repository: mincho-js/mincho
Length of output: 42
🏁 Script executed:
cat -n packages/oxc/eslint.config.jsRepository: mincho-js/mincho
Length of output: 176
🏁 Script executed:
find . -type f -path "*/eslint-config-custom/*" -name "*.ts" -o -path "*/eslint-config-custom/*" -name "*.js" 2>/dev/null | head -20Repository: mincho-js/mincho
Length of output: 224
🏁 Script executed:
rg -l "eslint-config-custom" --type json --type ts --type js | head -10Repository: mincho-js/mincho
Length of output: 396
🏁 Script executed:
cat -n configs/eslint-config-custom/eslint.config.typescript.jsRepository: mincho-js/mincho
Length of output: 2543
🏁 Script executed:
cat -n packages/oxc/tsconfig.jsonRepository: mincho-js/mincho
Length of output: 608
ESLint configuration must reference tsconfig.eslint.json—currently orphaned.
The tsconfig.eslint.json file is created with appropriate settings (allowJs: true, separate cache directory), but it's not integrated into ESLint. The shared ESLint config in configs/eslint-config-custom/eslint.config.typescript.js hardcodes project: ["tsconfig.json"] (line 32), and since the base packages/oxc/tsconfig.json lacks allowJs: true, ESLint will not type-check JavaScript files like eslint.config.js.
Update packages/oxc/eslint.config.js to pass a configuration override:
import { eslintConfig } from "eslint-config-custom/typescript";
export default eslintConfig({
languageOptions: {
parserOptions: {
project: ["tsconfig.eslint.json"]
}
}
});🤖 Prompt for AI Agents
In packages/oxc/eslint.config.js (around the top of the file), the ESLint config
currently references the default tsconfig and so does not use the
package-specific tsconfig.eslint.json; update the file to call eslintConfig from
eslint-config-custom/typescript with an override that sets
languageOptions.parserOptions.project to ["tsconfig.eslint.json"] so ESLint will
use the package tsconfig for type-aware checks (including allowJs) — import the
eslintConfig and return eslintConfig({ languageOptions: { parserOptions: {
project: ["tsconfig.eslint.json"] } } }).
…for improved ESLint integration
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit 97423dbaaa390bb7e8e31919960b320ac2d81eb6 (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Mon Dec 8 22:32:50 2025 +0900
Chore: Add tsconfig.eslint.json path to OXC TypeScript configuration for improved ESLint integrationIt is possible to fast forward |
….json for OXC TypeScript configuration
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit 878b60a6fd65edc043a6818972744cd325116ecf (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Mon Dec 8 23:00:54 2025 +0900
Feat: Enhance minchoTransform to support CSS extraction and style imports handlingIt is possible to fast forward |
| /import\s+\{[^}]*\bstyled\b[^}]*\}\s+from\s+["']@mincho-js\/react["']/.test( | ||
| code | ||
| ); |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data High
| /import\s+\{[^}]*\bstyle\b[^}]*\}\s+from\s+["']@mincho-js\/css["']/.test( | ||
| code | ||
| ); |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data High
| /import\s+\{([^}]*)\}\s+from\s+["']@mincho-js\/css["'];?\s*/g; | ||
| let styleMatch; | ||
|
|
||
| while ((styleMatch = styleImportRegex.exec(code)) !== null) { |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data High
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/oxc/src/mincho-transform.ts (3)
37-68: Import removed before verifying transformations succeed (previously flagged)This issue was identified in prior reviews: the
styledimport is removed (lines 62-68) before confirming that anystyled(...)calls were actually transformed (lines 105-168). If no calls match the expected pattern—for example,styled(Button, stylesVar)where the second argument isn't an object literal—the code will fail at runtime withReferenceError: styled is not defined.The previous review recommended tracking a
transformedCountand only removing the import if at least one call was rewritten. That fix has not yet been applied.Based on past review comments.
Also applies to: 105-168
39-41: Polynomial regex patterns remain vulnerable to ReDoS (previously flagged)CodeQL and previous reviews identified that the import-matching regexes (lines 39, 45, 64, 73) use nested quantifiers that can trigger polynomial backtracking on malicious input (e.g., strings with many repetitions of
import {{). Since this code processes arbitrary user source files, the risk is real.The earlier suggestion was to replace regex-based import detection with AST-based import inspection using the existing
parseSynccall. This would eliminate the ReDoS vector entirely and provide more reliable import detection.Based on past review comments and static analysis hints.
Also applies to: 45-47, 64-64, 73-73, 76-76
271-290: Dynamic regex infindFunctionCalls(previously flagged)As noted in earlier reviews, constructing a
RegExpfromfunctionName(line 276) triggers static analysis warnings. In current usage this is safe because only hardcoded values ("styled","style") are passed, but the helper could be misused if extended.The previous suggestion was to validate
functionNamematches/^[A-Za-z_$][A-Za-z0-9_$]*$/or replace the regex with a simpleindexOfloop to eliminate the vector entirely.Based on past review comments and static analysis hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/oxc/src/mincho-transform.ts(1 hunks)packages/oxc/tsconfig.eslint.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/oxc/tsconfig.eslint.json
🧰 Additional context used
🪛 ast-grep (0.40.0)
packages/oxc/src/mincho-transform.ts
[warning] 275-275: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b${functionName}\\s*\\(, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 GitHub Check: CodeQL
packages/oxc/src/mincho-transform.ts
[failure] 39-41: Polynomial regular expression used on uncontrolled data
This regular expression that depends on library input may run slow on strings starting with 'import {{' and with many repetitions of 'import {{'.
[failure] 45-47: Polynomial regular expression used on uncontrolled data
This regular expression that depends on library input may run slow on strings starting with 'import {{' and with many repetitions of 'import {{'.
[failure] 66-66: Polynomial regular expression used on uncontrolled data
This regular expression that depends on library input may run slow on strings starting with 'import {{' and with many repetitions of 'import {{'.
[failure] 76-76: Polynomial regular expression used on uncontrolled data
This regular expression that depends on library input may run slow on strings starting with 'import {{' and with many repetitions of 'import {{|'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: typecheck
🔇 Additional comments (1)
packages/oxc/src/mincho-transform.ts (1)
247-256: Source map generation properly implementedThe transform now correctly generates and returns source maps using
MagicString.generateMap()with appropriate options (includeContent,hires). This addresses the concern raised in previous reviews about returningmap: null.
| function findFirstCommaOutsideParens(code: string, startIndex: number): number { | ||
| let depth = 0; | ||
| let inString = false; | ||
| let stringChar = ""; | ||
|
|
||
| for (let i = startIndex; i < code.length; i++) { | ||
| const char = code[i]; | ||
| const prevChar = i > 0 ? code[i - 1] : ""; | ||
|
|
||
| if (prevChar === "\\" && (char === '"' || char === "'")) { | ||
| continue; | ||
| } | ||
|
|
||
| if ((char === '"' || char === "'") && !inString) { | ||
| inString = true; | ||
| stringChar = char; | ||
| continue; | ||
| } | ||
|
|
||
| if (inString && char === stringChar) { | ||
| inString = false; | ||
| continue; | ||
| } | ||
|
|
||
| if (inString) continue; | ||
|
|
||
| if (char === "(") { | ||
| depth++; | ||
| } else if (char === ")") { | ||
| depth--; | ||
| } else if (char === "," && depth === 0) { | ||
| return i; | ||
| } | ||
| } | ||
|
|
||
| return -1; | ||
| } |
There was a problem hiding this comment.
Template literal handling missing in findFirstCommaOutsideParens
Unlike findMatchingClosingParen (lines 324-336) and findMatchingBrace (lines 434-437), this function doesn't track template literal state. If a styled() call contains a template string before the first comma, commas inside the template will be incorrectly treated as argument separators:
styled(Button, { prop: `value,with,commas` })
// ^ incorrectly detected as first commaApply this diff to handle template literals consistently:
function findFirstCommaOutsideParens(code: string, startIndex: number): number {
let depth = 0;
let inString = false;
let stringChar = "";
+ let inTemplate = false;
for (let i = startIndex; i < code.length; i++) {
const char = code[i];
const prevChar = i > 0 ? code[i - 1] : "";
- if (prevChar === "\\" && (char === '"' || char === "'")) {
+ if (prevChar === "\\" && (char === '"' || char === "'" || char === "`")) {
continue;
}
- if ((char === '"' || char === "'") && !inString) {
+ if ((char === '"' || char === "'") && !inString && !inTemplate) {
inString = true;
stringChar = char;
continue;
}
if (inString && char === stringChar) {
inString = false;
continue;
}
+ if (char === "`" && !inString) {
+ inTemplate = !inTemplate;
+ continue;
+ }
+
- if (inString) continue;
+ if (inString || inTemplate) continue;
if (char === "(") {
depth++;
} else if (char === ")") {
depth--;
} else if (char === "," && depth === 0) {
return i;
}
}
return -1;
}🤖 Prompt for AI Agents
In packages/oxc/src/mincho-transform.ts around lines 356 to 392, the function
findFirstCommaOutsideParens fails to account for template literals so commas
inside backtick strings (and inside their ${...} expressions) are misinterpreted
as argument separators; update the function to track template state similarly to
the other helpers by adding an inTemplate boolean, handling the backtick (`)
start/end while respecting escapes, and when inside a template enter/exit ${...}
expressions by tracking a templateExpressionDepth counter so that braces inside
${...} don't end the template prematurely; ensure you ignore commas while
inString or inTemplate (and ignore text inside ${...} until its matching brace)
and decrement/increment depth for parentheses only when not inside a
string/template expression, so the first comma returned is truly outside
parentheses and template literals.
…ons and exports structure
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit fa61197e6fe1ec1ba817a402ed5b2dfe25a9846c (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Mon Dec 8 23:17:48 2025 +0900
Chore: Refactor package.json in debug-log to streamline type definitions and exports structureIt is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/debug-log/package.json (1)
23-24: Consider removing redundanttypingsfield.Both
typesandtypingspoint to the same location. Thetypesfield is the modern standard;typingsis legacy. Keeping both adds minimal maintenance burden but reduces clarity. If backwards compatibility with older tooling isn't a concern, consider removing line 24.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/debug-log/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: typecheck
🔇 Additional comments (2)
packages/debug-log/package.json (2)
29-32: Explicit export conditions follow best practices.The expanded exports map with explicit
types,import,require, anddefaultentries provides clear, unambiguous module resolution for both TypeScript and dual-package (ESM/CJS) scenarios. The.mjsand.cjsextensions are correctly distinguished.
36-37: Verify the intent of./importMetadefault export.Pointing a
defaultexport to a.d.tsfile is unconventional; type definition files are typically not runtime exports. Please confirm:
- Is
./importMetaintended only for type imports (e.g.,import type { ... } from "@mincho-js/debug-log/importMeta")?- If so, consider clarifying the export with a comment or documentation, as consumers may assume
defaultis a runtime value.- If a runtime value is needed alongside types, ensure the implementation is available.
… Vite configuration
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit e2603f6fdca916d47c71eebda0c6f2b4465a1be8 (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Mon Dec 8 23:45:20 2025 +0900
Chore: Update NodeBuilder to include 'ACTIONS' runtime environment in Vite configurationIt is possible to fast forward |
black7375
left a comment
There was a problem hiding this comment.
Thank you. This PR will be a huge leap forward for the project.
.gitignore
Outdated
| # Keep root workspace manifests tracked while ignoring fixture manifests | ||
| configs/*/**/package.json | ||
| packages/*/**/package.json | ||
| examples/*/**/package.json | ||
| !configs/*/package.json | ||
| !packages/*/package.json | ||
| !examples/*/package.json |
There was a problem hiding this comment.
How about like this?
I think the meaning will be clearer.
| # Keep root workspace manifests tracked while ignoring fixture manifests | |
| configs/*/**/package.json | |
| packages/*/**/package.json | |
| examples/*/**/package.json | |
| !configs/*/package.json | |
| !packages/*/package.json | |
| !examples/*/package.json | |
| # Keep root workspace manifests tracked while ignoring fixture manifests | |
| configs/*/*/**/package.json | |
| packages/*/*/**/package.json | |
| examples/*/*/**/package.json |
| ".": { | ||
| "import": { | ||
| "types": "./dist/esm/index.d.ts", | ||
| "default": "./dist/esm/index.mjs" | ||
| }, | ||
| "require": { | ||
| "types": "./dist/cjs/index.d.ts", | ||
| "default": "./dist/cjs/index.cjs" | ||
| } | ||
| "types": "./dist/esm/index.d.ts", | ||
| "import": "./dist/esm/index.mjs", | ||
| "require": "./dist/cjs/index.cjs", | ||
| "default": "./dist/esm/index.mjs" | ||
| }, |
There was a problem hiding this comment.
Any idea why this changed?
| @@ -0,0 +1,358 @@ | |||
| import { describe, it, expect, beforeEach, afterEach } from "vitest"; | |||
There was a problem hiding this comment.
Since tests run in parallel by default, they can influence each other.
Also, how about try using bench, since it makes it easier to view statistics?
…mproved package management and path resolution
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit 5ba2bcf9e8c8639d498fcb46416b900e216bd32b (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Sat Jan 10 06:59:58 2026 +0900
Chore: Refactor TypeScript configurations and update .gitignore for improved package management and path resolutionIt is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @examples/react-babel/tsconfig.app.json:
- Around line 29-35: The tsconfig uses moduleResolution: "bundler" and an
overly-broad catch-all path mapping "*" which is unnecessary and can mask
resolution intent; remove the "*" : ["../../*"] entry from the "paths" block and
add an explicit "baseUrl": "." at the top-level of tsconfig.app.json so the
@mincho-js/* aliases (e.g., "@mincho-js/react", "@mincho-js/css",
"@mincho-js/vite") continue to resolve to ../../packages/... while keeping
module resolution explicit and avoiding the wildcard mapping.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.gitignoreexamples/react-babel/tsconfig.app.jsonpackages/debug-log/package.jsonpackages/integration/tsconfig.lib.jsonpackages/oxc/tsconfig.lib.jsonpackages/oxc/tsconfig.test.json
✅ Files skipped from review due to trivial changes (2)
- packages/oxc/tsconfig.test.json
- packages/oxc/tsconfig.lib.json
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- packages/debug-log/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-03T11:16:32.528Z
Learnt from: black7375
Repo: mincho-js/mincho PR: 134
File: packages/transform-to-vanilla/src/transform.ts:49-51
Timestamp: 2024-12-03T11:16:32.528Z
Learning: In projects that target both CommonJS and ES modules, adjusting the TypeScript `module` option may not be feasible. Therefore, using `ts-ignore` comments to suppress TypeScript errors related to `import.meta` is acceptable.
Applied to files:
examples/react-babel/tsconfig.app.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: typecheck
🔇 Additional comments (1)
packages/integration/tsconfig.lib.json (1)
5-8: Good addition for cleaner imports.The path alias
"@/*"is properly integrated into the build pipeline via thevite-tsconfig-pathsplugin invite-config-custom, which automatically resolves TypeScript path mappings. The alias is actively used in the source code (e.g.,packages/integration/src/index.ts), and thebaseUrlchange from"./"to"."is a standard practice.
| /* Path Aliases for Workspace Packages */ | ||
| "baseUrl": "../../", | ||
| "paths": { | ||
| "@mincho-js/react": ["packages/react/src"], | ||
| "@mincho-js/css": ["packages/css/src"], | ||
| "@mincho-js/vite": ["packages/vite/src"] | ||
| "*": ["../../*"], | ||
| "@mincho-js/react": ["../../packages/react/src"], | ||
| "@mincho-js/css": ["../../packages/css/src"], | ||
| "@mincho-js/vite": ["../../packages/vite/src"] | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Inspecting tsconfig.app.json ==="
cat -n examples/react-babel/tsconfig.app.json 2>/dev/null || echo "File not found"
echo
echo "=== All tsconfig files in examples/react-babel ==="
fd -a '^tsconfig.*\.json$' examples/react-babel -x echo {} 2>/dev/null || true
echo
echo "=== Check for extends and baseUrl in tsconfig.app.json ==="
rg -n '"extends"|"baseUrl"' examples/react-babel/tsconfig.app.json 2>/dev/null || echo "No extends or baseUrl found"
echo
echo "=== Check what tsconfig.app.json extends from (if anything) ==="
grep -i extends examples/react-babel/tsconfig.app.json || echo "No extends directive"
echo
echo "=== Inspect parent tsconfig in examples/ ==="
fd -a '^tsconfig.*\.json$' examples -maxdepth 1 -x echo {} 2>/dev/null || true
echo
echo "=== Imports in examples/react-babel/src - look for bare/monorepo paths ==="
find examples/react-babel/src -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) 2>/dev/null | head -20Repository: mincho-js/mincho
Length of output: 1991
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== All import statements in examples/react-babel/src ==="
rg -n "^import|^export.*from" examples/react-babel/src/ -t ts -t tsx --max-count=50 2>/dev/null || true
echo
echo "=== Check for subpath imports of @mincho-js/* ==="
rg -n '@mincho-js/[^"'\'']+/' examples/react-babel/src/ -t ts -t tsx 2>/dev/null || echo "No subpath imports found"
echo
echo "=== Check what non-relative, non-@scope imports exist ==="
rg -n 'from\s+["\x27](?![./])([a-z])' examples/react-babel/src/ -t ts -t tsx --max-count=30 2>/dev/null || trueRepository: mincho-js/mincho
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check tsconfig hierarchy - root and examples ==="
if [ -f tsconfig.json ]; then
echo "Root tsconfig.json exists"
grep -E '"extends"|"baseUrl"' tsconfig.json || echo "No extends/baseUrl in root"
fi
echo
echo "=== Check if examples/tsconfig.json exists ==="
if [ -f examples/tsconfig.json ]; then
echo "examples/tsconfig.json content:"
cat -n examples/tsconfig.json
fi
echo
echo "=== Verify structure of examples/react-babel ==="
ls -la examples/react-babel/ | grep -E "tsconfig|src" || trueRepository: mincho-js/mincho
Length of output: 514
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== examples/react-babel/tsconfig.json ==="
cat -n examples/react-babel/tsconfig.json
echo
echo "=== Source files in examples/react-babel/src ==="
for file in examples/react-babel/src/*.tsx examples/react-babel/src/*.ts; do
if [ -f "$file" ]; then
echo "--- $file ---"
cat -n "$file"
echo
fi
doneRepository: mincho-js/mincho
Length of output: 2687
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== TypeScript paths behavior verification ==="
echo "TS docs: paths without baseUrl are typically ignored or baseUrl defaults to '.'"
echo "Checking if moduleResolution affects this..."
echo
echo "=== Check tsconfig.node.json ==="
cat -n examples/react-babel/tsconfig.node.json
echo
echo "=== Verify what import patterns actually exist ==="
rg -n 'import\s+' examples/react-babel/src/ -A 1 2>/dev/null || echo "No imports found"Repository: mincho-js/mincho
Length of output: 1467
Configuration is functional but could be cleaner
The tsconfig.app.json uses moduleResolution: "bundler", which implicitly treats baseUrl as "." when not explicitly set, so the @mincho-js/* path aliases work as intended. However, the "*": ["../../*"] catch-all is unnecessarily broad. While it doesn't currently mask missing external dependencies (since packages like react and react-dom resolve to node_modules first), removing it and adding an explicit baseUrl would improve clarity and reduce potential confusion:
Suggested tightening
{
"compilerOptions": {
+ "baseUrl": ".",
/* Path Aliases for Workspace Packages */
"paths": {
- "*": ["../../*"],
"@mincho-js/react": ["../../packages/react/src"],
"@mincho-js/css": ["../../packages/css/src"],
"@mincho-js/vite": ["../../packages/vite/src"]
},Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @examples/react-babel/tsconfig.app.json around lines 29 - 35, The tsconfig
uses moduleResolution: "bundler" and an overly-broad catch-all path mapping "*"
which is unnecessary and can mask resolution intent; remove the "*" :
["../../*"] entry from the "paths" block and add an explicit "baseUrl": "." at
the top-level of tsconfig.app.json so the @mincho-js/* aliases (e.g.,
"@mincho-js/react", "@mincho-js/css", "@mincho-js/vite") continue to resolve to
../../packages/... while keeping module resolution explicit and avoiding the
wildcard mapping.
…st-diff for improved diff functionality and streamline dependencies
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit f073162d8b2f8ea18b9983eb5e819b892ccbf7de (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Sat Jan 10 08:09:24 2026 +0900
Chore: Update package.json and debug-log to replace deep-diff with jest-diff for improved diff functionality and streamline dependenciesIt is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @package.json:
- Line 61: The package.json currently pins "eslint" to "9.39.0", which is a
buggy release; update the dependency to either "9.39.1" (or later) or change the
version spec to a range like "^9.39.0" so patches are allowed; modify the
"eslint" entry in package.json accordingly and run install/lockfile update
(e.g., npm install or pnpm install) to regenerate your lockfile.
In @packages/debug-log/src/index.ts:
- Line 4: The code assumes jest-diff's diffObjects returns null for identical
values, but it actually returns the string "Compared values have no visual
difference."; update the logic around the call that uses diffObjects (the
variable difference and the block that checks difference === null) to instead
detect identical objects before diffing — e.g., import and use
util.isDeepStrictEqual (or another reliable deep-equality check) to compare
expected and actual and short-circuit to the "Same Contents" path when they are
equal, otherwise call diffObjects and handle its string output (or explicitly
check for "Compared values have no visual difference.") so the identical-case
branch becomes reachable.
🧹 Nitpick comments (1)
package.json (1)
15-15: Remove redundant vite-config-custom builds from the root build and test scripts.The
buildandtestscripts explicitly rebuildvite-config-custombefore running Turbo tasks, but this is already handled by Turbo's dependency resolution ("dependsOn": ["^build"]in turbo.json). Since multiple packages depend on vite-config-custom as a workspace dependency, Turbo will automatically build it first with caching support.The explicit
yarn workspace vite-config-custom buildcommands bypass Turbo's caching, causing unnecessary rebuilds on every invocation even when the config hasn't changed. Thepostinstallhook already ensures the initial build is present, and Turbo's task graph will respect the dependency.Consider removing these explicit rebuilds and relying on Turbo's built-in dependency management for cleaner, more efficient CI/CD pipelines.
Also applies to: 28-28
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
package.jsonpackages/debug-log/package.jsonpackages/debug-log/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/debug-log/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: typecheck
- GitHub Check: build
🔇 Additional comments (2)
packages/debug-log/src/index.ts (2)
114-131: Well-structured refactoring with centralized diff logic.The new
printDiffhelper effectively centralizes the diff computation and output logic, improving maintainability. The handling of identical objects (line 123-126) and differences (lines 128-130) is clear and appropriate.Note that the diff output format has changed from
deep-difftojest-diff, which may affect the readability of differences in debug output. The new format uses jest's standard diff notation (with symbols like ✏️, ➕, ❌ as shown in the test output comments), which is widely recognized but different from the previous implementation.
133-139: LGTM: Function signature preserved with improved implementation.The refactoring maintains backward compatibility by preserving the existing function overloads while delegating to the new
printDiffhelper. The type assertions are safe given the function signature constraints.
| "@yarnpkg/types": "^4.0.1", | ||
| "c8": "^10.1.3", | ||
| "eslint": "^9.38.0", | ||
| "eslint": "9.39.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ESLint 9.39.0 known issues compatibility problems release notes
💡 Result:
Short summary — ESLint v9.39.0 (minor) changed some rule error locations and accidentally started passing a second argument to rule visitor functions, which broke some third‑party plugins (notably TypeScript-related plugins). The project published v9.39.1 to restore the prior single‑argument visitor behavior; related type/plugin breakage is tracked in GitHub issues. (github.com)
Citations:
🏁 Script executed:
# Search for any documentation or comments about version pinning in the repository
rg -i "eslint|version.*pin|exact.*version" package.json package-lock.json README.md -A 2 -B 2 || true
# Check git history for the change if available
git log -p --all -- package.json | grep -A 10 -B 10 "eslint.*9.39" | head -50 || trueRepository: mincho-js/mincho
Length of output: 1604
Update ESLint to 9.39.1 or use range specifier ^9.39.0.
ESLint v9.39.0 introduced a breaking change that accidentally passed a second argument to rule visitor functions, breaking third-party plugins (notably TypeScript-related ones). This issue was fixed in ESLint v9.39.1. The exact pinning to the buggy 9.39.0 should be either upgraded to 9.39.1 or later, or restored to ^9.39.0 to allow patch and minor updates.
🤖 Prompt for AI Agents
In @package.json at line 61, The package.json currently pins "eslint" to
"9.39.0", which is a buggy release; update the dependency to either "9.39.1" (or
later) or change the version spec to a range like "^9.39.0" so patches are
allowed; modify the "eslint" entry in package.json accordingly and run
install/lockfile update (e.g., npm install or pnpm install) to regenerate your
lockfile.
| import colorize from "@pinojs/json-colorizer"; | ||
| import diff from "deep-diff"; | ||
| import prettifyDeepDiff from "@mincho-js/pretify-deep-diff"; | ||
| import { diff as diffObjects } from "jest-diff"; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
jest-diff function return value identical objects null
💡 Result:
Short answer: jest-diff's diff(a, b) does NOT always return null for identical objects. It returns the string "Compared values have no visual difference." when the values are referentially identical (Object.is) or serialize the same; it returns null only when either argument is an asymmetric matcher. [1][2]
References:
- npm jest-diff README (edge-case behavior). [1]
- Implementation/notes showing Object.is check and asymmetric-matcher early return. [2]
🏁 Script executed:
# Find the index.ts file and examine the relevant code sections
find . -name "index.ts" -path "*/packages/debug-log/src/*" -type f | head -5Repository: mincho-js/mincho
Length of output: 93
🏁 Script executed:
# Read the actual code to see the printDiff implementation and how jest-diff is being used
cat -n ./packages/debug-log/src/index.ts | sed -n '1,150p'Repository: mincho-js/mincho
Length of output: 5672
Fix jest-diff null check—it returns a string for identical objects, not null.
The code incorrectly assumes jest-diff's diff() function returns null for identical objects. According to jest-diff behavior, when compared values are identical (either referentially or after serialization), it returns the string "Compared values have no visual difference.", not null. The null return is reserved for asymmetric matcher cases.
This means line 123's check if (difference === null) will never execute for identical objects, causing the "Same Contents" path to be unreachable. Instead, identical objects will incorrectly print "Expected" and "Real" values alongside the difference string.
Correct the condition to check for the jest-diff string output, or use an alternative method to detect identical objects (e.g., deep equality check before calling diffObjects).
🤖 Prompt for AI Agents
In @packages/debug-log/src/index.ts at line 4, The code assumes jest-diff's
diffObjects returns null for identical values, but it actually returns the
string "Compared values have no visual difference."; update the logic around the
call that uses diffObjects (the variable difference and the block that checks
difference === null) to instead detect identical objects before diffing — e.g.,
import and use util.isDeepStrictEqual (or another reliable deep-equality check)
to compare expected and actual and short-circuit to the "Same Contents" path
when they are equal, otherwise call diffObjects and handle its string output (or
explicitly check for "Compared values have no visual difference.") so the
identical-case branch becomes reachable.
…r improved testing setup
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit 6ae70aa1136e5b8624ef5cc9f5c4192566979927 (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Sat Jan 10 08:25:25 2026 +0900
Chore: Add tsconfig.test.json path to OXC TypeScript configuration for improved testing setupIt is possible to fast forward |
…and improve cache management
|
Triggered from #281 by @sangkukbae. Checking if we can fast forward Target branch ( commit 236463ba62ad15c5ebb0bda3dc89e9d1e36e4eff (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Oct 18 00:00:00 2025 +0900
Fix: `theme` - composite valuePull request ( commit 56b3d6ed35185f82900ab9f024d58f0b1e04ac99 (pull_request/feat/oxc)
Author: sangkukbae <sangkukbae@gmail.com>
Date: Sat Jan 10 18:24:40 2026 +0900
Chore: Update TypeScript configurations in OXC to extend base config and improve cache managementIt is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/oxc/tsconfig.lib.json:
- Line 6: The tsconfig property tsBuildInfoFile is pointing to a malformed path
".cache/typescript/tsbuildinfo"; update the tsBuildInfoFile value to
"./.cache/typescript/tsconfig.lib.tsbuildinfo" in the
packages/oxc/tsconfig.lib.json so it includes the leading ./, the tsconfig.lib.
prefix to avoid conflicts, and the correct .tsbuildinfo filename extension.
🧹 Nitpick comments (2)
packages/oxc/tsconfig.test.json (1)
10-13: Optional: Remove redundant include pattern.Line 12 is redundant since line 11's
src/**/*.tspattern already matches all TypeScript files recursively, including those in__tests__directories.♻️ Simplify include patterns
"include": [ - "src/**/*.ts", - "src/**/__tests__/**/*.ts" + "src/**/*.ts" ],.github/workflows/ci.yml (1)
102-110: Watch cache size/eviction risk when caching**/distin build-and-test.Caching
dist/**/distcan balloon quickly (especially with examples/packages) and lead to slower cache save/restore or more frequent eviction. If CI time doesn’t improve, consider narrowing this to the specific package build outputs that are reused (or only caching tool caches like.cache/**).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/ci.ymlpackages/oxc/tsconfig.eslint.jsonpackages/oxc/tsconfig.jsonpackages/oxc/tsconfig.lib.jsonpackages/oxc/tsconfig.test.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/oxc/tsconfig.eslint.json
- packages/oxc/tsconfig.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: typecheck
🔇 Additional comments (3)
packages/oxc/tsconfig.test.json (1)
1-18: LGTM! Well-structured test configuration.The test tsconfig is properly set up with composite builds, isolated cache directories, and appropriate type includes for the test environment. The configuration follows monorepo best practices.
packages/oxc/tsconfig.lib.json (1)
12-12: No changes needed to the include pattern.The packages/oxc/src/ directory contains only
.tsfiles (8 total), not.tsxfiles. The include pattern"src/**/*.ts"correctly captures all source files. The"src/**/*.test.tsx"entry in the exclude pattern is defensive but not indicative of missing.tsxsources that should be included.Likely an incorrect or invalid review comment.
.github/workflows/ci.yml (1)
67-76: Cache key and glob patterns are properly configured.The custom
cache-diraction correctly composes the cache key asrunner.os-cache-[key]-[yarn.lock hash]-[day], which includes both OS and lockfile hash, and passes glob patterns through toactions/cache@v4, which natively supports them. No action needed.
| "compilerOptions": { | ||
| "composite": true, | ||
| "declaration": true, | ||
| "tsBuildInfoFile": ".cache/typescript/tsbuildinfo", |
There was a problem hiding this comment.
Incorrect tsBuildInfoFile path.
The path ".cache/typescript/tsbuildinfo" appears malformed. Based on the AI summary and TypeScript conventions, it should be "./.cache/typescript/tsconfig.lib.tsbuildinfo":
- Missing leading
./ - Missing
tsconfig.lib.prefix to avoid conflicts with other configs - File extension should be
.tsbuildinfo(with the dot)
This could cause build failures or conflicts with other packages.
🔧 Proposed fix
- "tsBuildInfoFile": ".cache/typescript/tsbuildinfo",
+ "tsBuildInfoFile": "./.cache/typescript/tsconfig.lib.tsbuildinfo",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "tsBuildInfoFile": ".cache/typescript/tsbuildinfo", | |
| "tsBuildInfoFile": "./.cache/typescript/tsconfig.lib.tsbuildinfo", |
🤖 Prompt for AI Agents
In @packages/oxc/tsconfig.lib.json at line 6, The tsconfig property
tsBuildInfoFile is pointing to a malformed path ".cache/typescript/tsbuildinfo";
update the tsBuildInfoFile value to
"./.cache/typescript/tsconfig.lib.tsbuildinfo" in the
packages/oxc/tsconfig.lib.json so it includes the leading ./, the tsconfig.lib.
prefix to avoid conflicts, and the correct .tsbuildinfo filename extension.
| paths: | | ||
| .cache | ||
| **/.cache | ||
| dist | ||
| **/dist |
There was a problem hiding this comment.
Does dist also need caching?
| "compilerOptions": { | ||
| "composite": true | ||
| }, |
| "composite": true, | ||
| "noEmit": false, | ||
| "declaration": true, | ||
| "emitDeclarationOnly": true, |
There was a problem hiding this comment.
I think that the node type should not be emitted, as it is only used for type checking for build purposes.
| "*": ["../../*"], | ||
| "@mincho-js/react": ["../../packages/react/src"], | ||
| "@mincho-js/css": ["../../packages/css/src"], | ||
| "@mincho-js/vite": ["../../packages/vite/src"] |
There was a problem hiding this comment.
I think we can remove them all.
| "url": "https://github.com/mincho-js/mincho/issues" | ||
| }, | ||
| "homepage": "https://github.com/mincho-js/mincho/tree/main/packages/debug-log#readme", | ||
| "types": "./dist/esm/index.d.ts", |
There was a problem hiding this comment.
This config overlaps to typings.
| import colorize from "@pinojs/json-colorizer"; | ||
| import diff from "deep-diff"; | ||
| import prettifyDeepDiff from "@mincho-js/pretify-deep-diff"; | ||
| import { diff as diffObjects } from "jest-diff"; |
There was a problem hiding this comment.
Can you please show me the screen that works when it changes?
| **Performance Improvements**: | ||
| - 🚀 10-15x faster builds | ||
| - 💾 60% less memory usage | ||
| - ⚡ Faster hot module replacement | ||
| - 🎯 Same bundle output |
There was a problem hiding this comment.
Can we hanve benchmark after merged?
Description
Replaces Babel with OXC for 10-15x faster build performance. Supports Mincho APIs and maintains full compatibility with existing Babel workflows.
Key Changes
New
@mincho-js/oxcpackageVite plugin enhancement
minchoOxcVitePlugin()(can be used alongside existing Babel plugin)Integration layer updates
compileWithOxc(),oxcTransform()functionsExample project
examples/react-oxcRelated Issue
Summary by CodeRabbit
New Features
Documentation
Tests
Dependency Updates
✏️ Tip: You can customize this high-level summary in your review settings.
Additional context
Checklist