Skip to content

Conversation

@sergeyk-symbiotic
Copy link
Contributor

@sergeyk-symbiotic sergeyk-symbiotic commented Dec 17, 2025

PR Type

Bug fix


Description

  • Fix action entry to .cjs

  • Remove forced .js output naming

  • Disable code splitting for single bundle


Diagram Walkthrough

flowchart LR
  TSUP["tsup.config.ts updated"] -- "remove .js override, add splitting=false" --> BUNDLE["CJS-compatible bundle"]
  ACTION["action.yml updated"] -- "main -> dist/index.cjs" --> RUNTIME["GitHub Actions runtime"]
  BUNDLE -- "loaded by" --> RUNTIME
Loading

File Walkthrough

Relevant files
Bug fix
tsup.config.ts
Adjust tsup to emit single CJS-compatible bundle                 

tsup.config.ts

  • Remove format: ["cjs"] and custom outExtension
  • Add splitting: false to ensure single bundle
  • Keep bundling, minify, treeshake settings
+1/-4     
action.yml
Update action entry to .cjs file                                                 

action.yml

  • Point runs.main to dist/index.cjs
  • Maintain node24 runtime configuration
+1/-1     

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Build Output Assumptions

Verify that removing the explicit outExtension and relying on default CJS output actually produces dist/index.cjs with the expected filename in all environments (CI/local). Ensure the generated file path matches action.yml and that the bundler does not emit .js instead.

export default defineConfig({
    entry: ["src/index.ts"],
    noExternal: [/.*/], // Bundle all dependencies
    target: "node24",
    clean: true,
    minify: true,
    treeshake: true,
    splitting: false,
Splitting Disabled

Confirm that disabling code splitting does not inadvertently increase bundle size or remove required dynamic imports behavior. Validate that the single-file bundle still works for the action and includes all dependencies given noExternal: [/.*/].

export default defineConfig({
    entry: ["src/index.ts"],
    noExternal: [/.*/], // Bundle all dependencies
    target: "node24",
    clean: true,
    minify: true,
    treeshake: true,
    splitting: false,
Runtime Compatibility

Ensure GitHub Actions using: node24 supports loading a .cjs entry without additional type configuration in package.json. Double-check that the path dist/index.cjs exists after build and is packaged for release.

runs:
  using: node24
  main: dist/index.cjs

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Align entry with emitted file

Verify the bundled filename actually emits as .cjs from the build. Align tsup output
to produce index.cjs to prevent runtime failures where the action runner can’t find
the entry file.

action.yml [45]

+main: dist/index.cjs
 
-
Suggestion importance[1-10]: 5

__

Why: The snippet main: dist/index.cjs matches line 45 and the suggestion is to verify the emitted filename, which is reasonable but only a request to verify. It offers limited concrete change and thus moderate impact.

Low
Ensure CommonJS output explicitly

Disabling code splitting can hide dynamic import issues but may inflate bundle size.
If dynamic imports aren’t used, keep this; otherwise, explicitly configure output as
CommonJS while allowing splitting for better performance.

tsup.config.ts [10]

+format: ["cjs"],
 splitting: false,
Suggestion importance[1-10]: 3

__

Why: The existing code line splitting: false, is correctly located at line 10, but adding format: ["cjs"], contradicts the PR that removed format configuration and switched the action to .cjs. The advice is low-impact and partially misaligned with the PR intent; also the improved_code adds a new key not present in the existing snippet.

Low

@sergey-symbiotic sergey-symbiotic merged commit 2142119 into main Dec 17, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants