Use import statement instead of require() - #120
Conversation
The `import` syntax is the modern standard in JavaScript so there is no reason to use Node.js's solution from before `import` was added to the JavaScript language specification.
There was a problem hiding this comment.
Pull request overview
This PR updates the action’s source to use ES module import syntax instead of CommonJS require(), as a step toward an eventual ESM migration, and refreshes the bundled dist/ output accordingly.
Changes:
- Replace
require()usage withimportinsrc/main.jsandsrc/index.js - Rebuild the bundled action output in
dist/index.js - Bump the package version to
1.3.0
Reviewed changes
Copilot reviewed 3 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/main.js |
Switches to import statements for action dependencies |
src/index.js |
Switches entrypoint to import the run function |
package.json |
Version bump to 1.3.0 |
package-lock.json |
Lockfile version alignment with package.json bump |
dist/index.js |
Updated ncc/webpack bundle reflecting source module changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import * as core from "@actions/core"; | ||
| import { toolVersions } from "./versions"; |
There was a problem hiding this comment.
src/main.js now uses ESM import syntax but still exports via module.exports. When bundled by ncc/webpack this is treated as an ES module and the generated runtime decorates module.exports to throw, causing the action to fail at runtime. Convert this module to ESM exports (e.g., export { run } / export async function run) or revert back to CommonJS require/module.exports consistently.
| * The entrypoint for the action. | ||
| */ | ||
| const { run } = require("./main"); | ||
| import { run } from "./main"; |
There was a problem hiding this comment.
src/index.js imports a named export run from ./main, but main.js currently exports via module.exports = { run }. With the new ESM-style imports this interop is fragile and, as currently bundled, results in the generated code treating main as an ES module while it still assigns module.exports. Make main.js an ES module export (and import accordingly), or keep both files in CommonJS. Also, if the intent is to run these sources as native ESM later, Node will require explicit .js extensions in relative imports.
| import { run } from "./main"; | |
| const { run } = require("./main"); |
21e5ef4 to
a5f4f07
Compare
a5f4f07 to
08f8e1e
Compare
The `export` syntax is the modern standard in JavaScript and is necessary to convert the action to an ECMAScript module.
This is done by specifying `"type": "module"` in the `package.json` file.
Generate the distribution files by running `npm run package`. This is necessary for changes to the codebase to take effect.
97aa373 to
76f51c7
Compare
🗣 Description
This pull request replaces use of Node.js's
require()function with the now standardimportsyntax.💭 Motivation and context
It is good to stay up-to-date. This is also necessary to move this action toward being an ECMAScript module.
🧪 Testing
I tested this locally with local-action
✅ Pre-approval checklist
bump_versionscript if this repository is versioned and the changes in this PR warrant a version bump.✅ Post-merge checklist