Skip to content

Fix GitHub issue #1127 in create-adapter#1255

Open
Apollon77 wants to merge 3 commits into
masterfrom
claude/fix-issue-1127-01PCszX1jdWVVsH7e1qptjyi
Open

Fix GitHub issue #1127 in create-adapter#1255
Apollon77 wants to merge 3 commits into
masterfrom
claude/fix-issue-1127-01PCszX1jdWVVsH7e1qptjyi

Conversation

@Apollon77

Copy link
Copy Markdown
Collaborator

Implements feature request from issue #1127 to support ECMAScript Modules (ESM) in generated adapters, providing better compatibility with modern dependencies.

Changes:

  • Add moduleType question in Code section with ESM as default (expert option)
  • Update Answers interface with moduleType property (esm | commonjs)
  • Modify package.json template to include "type": "module" for ESM
  • Update TypeScript main template with ESM imports and export patterns
  • Update JavaScript main template with ESM imports and export patterns
  • Configure tsconfig.json with ESM-specific compiler options (module: ES2022, moduleResolution: bundler)
  • Add comment in io-package.json noting ESM compatibility with compact mode (requires js-controller >= 6.0.11)

Benefits:

  • Prevents issues with ESM-only dependencies
  • Enables modern JavaScript features
  • Works with js-controller 6.0.11+ compact mode
  • Backward compatible via CommonJS option

Fixes #1127

PR Checklist:

  • Provide a meaningful description to this PR or mention which issues this fixes.
  • Ensure the project builds with npm run build
  • Add tests for your change. This includes negative tests (i.e. inputs that need to fail) as well as baseline tests (i.e. how should the directory structure look like?).
  • Run the test suite with npm test
  • If there are baseline changes, review them and make a separate commit for them with the comment "accept baselines" if they are desired changes
  • If you added a required option, also add it to the template creation (.github/create_templates.ts)
  • Add a detailed migration description to docs/updates explaining what the user needs to do when manually updating an existing project
  • Add your changes to CHANGELOG.md (referencing the migration description and this PR or the issue you fixed)

Description:
add description here

Implements feature request from issue #1127 to support ECMAScript Modules (ESM)
in generated adapters, providing better compatibility with modern dependencies.

Changes:
- Add moduleType question in Code section with ESM as default (expert option)
- Update Answers interface with moduleType property (esm | commonjs)
- Modify package.json template to include "type": "module" for ESM
- Update TypeScript main template with ESM imports and export patterns
- Update JavaScript main template with ESM imports and export patterns
- Configure tsconfig.json with ESM-specific compiler options (module: ES2022, moduleResolution: bundler)
- Add comment in io-package.json noting ESM compatibility with compact mode (requires js-controller >= 6.0.11)

Benefits:
- Prevents issues with ESM-only dependencies
- Enables modern JavaScript features
- Works with js-controller 6.0.11+ compact mode
- Backward compatible via CommonJS option

Fixes #1127
@Apollon77

Copy link
Copy Markdown
Collaborator Author

@copilot Please finish this PR. We need node.js minimum versions like the following for pure ESM adapters

"engines": {
    "node": ">=20.19.0 <22.0.0 || >=22.13.0"
  },

Additionally we want to also publish such a configuration as exaple and also have it tested. Please enhance this

Copilot AI commented Jan 10, 2026

Copy link
Copy Markdown
Contributor

@Apollon77 I've opened a new pull request, #1280, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI review requested due to automatic review settings January 10, 2026 16:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request implements ESM (ECMAScript Modules) support in the adapter generator to address GitHub issue #1127. The changes enable developers to create adapters using modern ESM syntax, providing better compatibility with ESM-only dependencies while maintaining backward compatibility through a CommonJS option.

Changes:

  • Added moduleType question in the Code section allowing selection between ESM and CommonJS (ESM is default for new projects, expert option)
  • Updated TypeScript and JavaScript main templates with conditional ESM/CommonJS import patterns and module exports
  • Modified tsconfig.json to include ESM-specific compiler options when ESM is selected
  • Added "type": "module" to package.json for ESM adapters

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/lib/core/questions.ts Added moduleType question with ESM/CommonJS choices and migration logic; extended Answers interface
templates/package.json.ts Conditionally adds "type": "module" field when ESM is selected
templates/tsconfig.json.ts Adds ESM-specific TypeScript compiler options (module: ES2022, moduleResolution: bundler)
templates/src/main.ts.ts Updates TypeScript template with ESM default import and conditional module export patterns
templates/main.js.ts Updates JavaScript template with ESM import syntax and conditional module export patterns
templates/io-package.json.ts Adds comment documenting ESM compatibility with compact mode (requires js-controller >= 6.0.11)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 48 to +58
"noEmit": true,
${
useESM
? `
// ESM module settings
"module": "ES2022",
"moduleResolution": "bundler",
`
: ""
}

Copilot AI Jan 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ESM module settings block is missing a trailing comma after "noEmit": true on line 49. When the ESM block is inserted, it creates invalid JSON because "noEmit": true is not followed by a comma before the next property. This will cause tsconfig.json parsing errors when ESM is selected.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

The ESM module settings block is missing a trailing comma after "noEmit": true on line 49. When the ESM block is inserted, it creates invalid JSON because "noEmit": true is not followed by a comma before the next property. This will cause tsconfig.json parsing errors when ESM is selected.

Comment thread templates/src/main.ts.ts
Comment on lines +197 to +199
if (import.meta.url === \`file://\${process.argv[1]}\`) {
new ${className}();
}

Copilot AI Jan 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ESM module detection condition using import.meta.url === \file://${process.argv[1]}`is incorrect. On Windows systems, the file URL format includes the drive letter, making this comparison unreliable across platforms. The correct pattern should useimport.meta.url === pathToFileURL(process.argv[1]).href` or convert process.argv[1] to a proper file URL using the url module's pathToFileURL function.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

The ESM module detection condition using import.meta.url === \file://${process.argv[1]}is incorrect. On Windows systems, the file URL format includes the drive letter, making this comparison unreliable across platforms. The correct pattern should useimport.meta.url === pathToFileURL(process.argv[1]).href or convert process.argv[1] to a proper file URL using the url module's pathToFileURL function.

Comment thread templates/main.js.ts
Comment on lines +207 to +209
if (import.meta.url === \`file://\${process.argv[1]}\`) {
new ${className}();
}

Copilot AI Jan 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ESM module detection condition using import.meta.url === \file://${process.argv[1]}`is incorrect. On Windows systems, the file URL format includes the drive letter, making this comparison unreliable across platforms. The correct pattern should useimport.meta.url === pathToFileURL(process.argv[1]).href` or convert process.argv[1] to a proper file URL using the url module's pathToFileURL function.

Copilot uses AI. Check for mistakes.
Comment thread templates/main.js.ts
${useESM ? `import utils from ${quote}@iobroker/adapter-core${quote};` : `const utils = require(${quote}@iobroker/adapter-core${quote});`}

// Load your modules here, e.g.:
// const fs = require(${quote}fs${quote});

Copilot AI Jan 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example comment on line 29 still uses CommonJS require syntax even when ESM is selected. When moduleType is "esm", this comment should be updated to use ESM import syntax instead, for example: // import * as fs from ${quote}fs${quote}; to match the pattern used in the main import above.

Suggested change
// const fs = require(${quote}fs${quote});
${useESM ? `// import * as fs from ${quote}fs${quote};` : `// const fs = require(${quote}fs${quote});`}

Copilot uses AI. Check for mistakes.
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
Co-authored-by: Ingo Fischer <github@fischer-ka.de>

Copilot AI commented Jan 10, 2026

Copy link
Copy Markdown
Contributor

@Apollon77 I've opened a new pull request, #1281, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI commented Jan 10, 2026

Copy link
Copy Markdown
Contributor

@Apollon77 I've opened a new pull request, #1282, to work on those changes. Once the pull request is ready, I'll request review from you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ESM module option

4 participants