Phase 2: convert lib/, index.js and tests to native ES modules#1530
Merged
Conversation
Convert all authored JavaScript from CommonJS to native ESM (atomic):
- lib/**/*.js (incl. action/ and runtime/), index.js, bin/rclnodejs-web.js,
rosocket/*: drop 'use strict', require->import with explicit .js/.cjs
extensions, module.exports->export; createRequire()/fileURLToPath shims
for the native loader and __dirname; dynamic import() for the
rate.js <-> index.js circular dependency.
- Add "type": "module" and a generated/package.json {"type":"commonjs"}
CJS-island guard (the generator emits it); add the same guard for the
vendored third_party/ref-napi.
- Generator/parser .cjs modules that require() now-ESM lib modules unwrap
the .default export (Node returns the namespace for require(ESM)).
- native_loader.js imports child_process as a namespace so test stubs of
execSync are honored.
- Convert all test/*.js to import.
- eslint.config.mjs: split ESM vs CJS sourceType and ignore build artifacts.
Verified: native build + full message regen + full mocha suite (1355
passing) + eslint 0 errors.
There was a problem hiding this comment.
Pull request overview
Phase 2 of the ESM migration: converts the authored JavaScript surface (core entrypoint, lib/, CLI/runtime, and tests) from CommonJS to native ES modules, and sets the package root to ESM via "type": "module".
Changes:
- Converted core modules and entrypoint from
require/module.exportstoimport/export(including addressing the prior circular-dependency workaround via ESM live bindings). - Converted test suite files to ESM and updated the Mocha runner to use async loading.
- Added/adjusted “CJS island” boundaries (e.g.,
third_party/ref-napi+ generated output scope) and updated lint config to distinguish ESM vs CJS globs.
Reviewed changes
Copilot reviewed 180 out of 181 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| third_party/ref-napi/package.json | Marks vendored ref-napi subtree as CommonJS under ESM package root. |
| third_party/ref-napi/lib/ref.js | Updates ref-napi to load the native addon via rclnodejs native loader. |
| test/utils.js | Converts shared test utilities to ESM exports/imports. |
| test/test-web-ws.js | Converts web runtime WS SDK tests to ESM imports. |
| test/test-web-http.js | Converts web runtime HTTP SDK tests to ESM imports. |
| test/test-web-cli.js | Converts web CLI tests to ESM and reconstructs __dirname. |
| test/test-wait-for-message.js | Converts waitForMessage tests to ESM. |
| test/test-validator.js | Converts validator tests to ESM. |
| test/test-utils.js | Converts utils unit tests to ESM imports. |
| test/test-unparsed-args.js | Converts unparsed-args tests to ESM. |
| test/test-type-description-service.js | Converts type description service tests to ESM. |
| test/test-timer.js | Converts timer tests to ESM. |
| test/test-time.js | Converts time tests to ESM. |
| test/test-time-source.js | Converts time source tests to ESM. |
| test/test-subscription.js | Converts subscription tests to ESM. |
| test/test-subscription-content-filter.js | Converts subscription content-filter tests to ESM. |
| test/test-spinning.js | Converts spinning tests to ESM. |
| test/test-spinning-oo.js | Converts OO spinning tests to ESM. |
| test/test-single-process.js | Converts single-process tests to ESM. |
| test/test-signals.js | Converts signal-handling tests to ESM and reconstructs __filename. |
| test/test-service.js | Converts service tests to ESM. |
| test/test-service-introspection.js | Converts service introspection tests to ESM imports/promisified exec. |
| test/test-serialization.js | Converts serialization tests to ESM + uses exports off default entry object. |
| test/test-serialization-modes.js | Converts serialization-mode tests to ESM. |
| test/test-security-related.js | Converts security-related tests to ESM (keeps CJS translator via .cjs). |
| test/test-runtime.js | Converts web runtime protocol tests to ESM. |
| test/test-rosocket.js | Converts rosocket tests to ESM imports. |
| test/test-rosidl-message-generator.js | Converts generator tests to ESM and adds createRequire/dirname helpers. |
| test/test-remove-ros-args.js | Converts removeROSArgs tests to ESM. |
| test/test-remapping.js | Converts remapping tests to ESM. |
| test/test-raw-pub-sub.js | Converts raw pub/sub tests to ESM. |
| test/test-rate.js | Converts rate tests to ESM. |
| test/test-qos-overriding-options.js | Converts QoS overriding options tests to ESM. |
| test/test-publisher.js | Converts publisher tests to ESM. |
| test/test-promise-gc.js | Converts promise-gc tests to ESM. |
| test/test-primitive-types.js | Converts primitive types tests to ESM (imports CJS module via .cjs). |
| test/test-primitive-msg-type-check.js | Converts primitive msg type-check tests to ESM. |
| test/test-pre-post-param-callbacks.js | Converts pre/post parameter callback tests to ESM. |
| test/test-parameters.js | Converts parameter tests to ESM. |
| test/test-parameter-watcher.js | Converts parameter watcher tests to ESM. |
| test/test-parameter-service.js | Converts parameter service tests to ESM. |
| test/test-parameter-event-handler.js | Converts parameter event handler tests to ESM. |
| test/test-parameter-client.js | Converts parameter client tests to ESM. |
| test/test-observable-subscription.js | Converts observable subscription tests to ESM and adjusts rxjs imports. |
| test/test-non-primitive-msg-type-check.js | Converts non-primitive msg type-check tests to ESM. |
| test/test-non-primitive-array.js | Converts non-primitive array tests to ESM. |
| test/test-node.js | Converts node tests to ESM and updates NodeOptions access. |
| test/test-node-options.js | Converts node options tests to ESM. |
| test/test-node-oo.js | Converts OO node tests to ESM and updates prettier import usage. |
| test/test-native-loader.js | Updates native loader tests for ESM singleton semantics and cache-busting imports. |
| test/test-msg-type-py-node.js | Converts Python msg type tests to ESM and reconstructs dirname. |
| test/test-msg-type-cpp-node.js | Converts C++ msg type tests to ESM and reconstructs dirname. |
| test/test-messsage-generation-overlay.js | Converts overlay generation tests to ESM and adds createRequire/dirname helpers. |
| test/test-message-validation.js | Converts message validation tests to ESM. |
| test/test-message-type.js | Converts message type tests to ESM and replaces late require with imported loader. |
| test/test-message-translator-primitive.js | Converts translator primitive tests to ESM. |
| test/test-message-translator-complex.js | Converts translator complex tests to ESM. |
| test/test-message-properties-validation.js | Converts message-properties validation tests to ESM. |
| test/test-message-object.js | Converts message object tests to ESM and uses createRequire where needed. |
| test/test-message-introspector.js | Converts message introspector tests to ESM. |
| test/test-message-info.js | Converts message info tests to ESM. |
| test/test-message-generation-bin.js | Converts message-generation bin tests to ESM imports. |
| test/test-logging.js | Converts logging tests to ESM. |
| test/test-logging-service.js | Converts logging service tests to ESM. |
| test/test-lifecycle.js | Converts lifecycle tests to ESM. |
| test/test-lifecycle-publisher.js | Converts lifecycle publisher tests to ESM. |
| test/test-interactive.js | Converts interactive tests to ESM and reconstructs dirname. |
| test/test-init-shutdown.js | Converts init/shutdown tests to ESM and uses cache-busting import for fresh entrypoint instance. |
| test/test-guard-condition.js | Converts guard condition tests to ESM. |
| test/test-graph.js | Converts graph tests to ESM. |
| test/test-fixed-array.js | Converts fixed-array tests to ESM. |
| test/test-extra-destroy-methods.js | Converts extra-destroy-methods tests to ESM. |
| test/test-expand-topic.js | Converts expand-topic tests to ESM. |
| test/test-existance.js | Converts existence tests to ESM. |
| test/test-event-handle.js | Converts event handler tests to ESM imports. |
| test/test-errors.js | Converts error tests to ESM. |
| test/test-distro.js | Converts distro tests to ESM. |
| test/test-disable-typedarray.js | Converts disable-typedarray tests to ESM. |
| test/test-destruction.js | Converts destruction tests to ESM and reconstructs dirname. |
| test/test-context.js | Converts context tests to ESM. |
| test/test-compound-msg-type-check.js | Converts compound msg type-check tests to ESM. |
| test/test-clock-sleep.js | Converts clock sleep tests to ESM and removes late requires. |
| test/test-clock-event.js | Converts clock event tests to ESM. |
| test/test-clock-change.js | Converts clock change tests to ESM and updates module export access. |
| test/test-clock-callback.js | Converts clock callback tests to ESM. |
| test/test-client.js | Converts client coverage tests to ESM. |
| test/test-bounded-sequences.js | Converts bounded-sequences tests to ESM. |
| test/test-async-client.js | Converts async client tests to ESM. |
| test/test-array.js | Converts array tests to ESM and reconstructs dirname. |
| test/test-array-data.js | Converts array-data tests to ESM. |
| test/test-action-uuid.js | Converts action UUID tests to ESM. |
| test/test-action-server.js | Converts action server tests to ESM and hoists imports previously inside functions. |
| test/test-action-graph.js | Converts action graph tests to ESM. |
| test/test-action-client.js | Converts action client tests to ESM. |
| test/publisher_setup.js | Converts publisher setup helper to ESM. |
| test/publisher_msg.js | Converts publisher message helper to ESM. |
| test/publisher_msg_jointstate.js | Converts publisher jointstate helper to ESM. |
| test/publisher_msg_header.js | Converts publisher header helper to ESM. |
| test/publisher_msg_colorrgba.js | Converts publisher colorrgba helper to ESM. |
| test/publisher_array_setup.js | Converts publisher array setup helper to ESM. |
| test/gc-on-exit.js | Removes CJS strict directive from GC helper (ESM context). |
| test/client_setup.js | Converts client setup helper to ESM. |
| test/array_generator.js | Converts array generator helper to ESM exports. |
| scripts/run_test.cjs | Updates Mocha runner to async-load ESM test files. |
| scripts/ros_distro.cjs | Adjusts ROS distro helper for ESM library imports. |
| rostsd_gen/index.cjs | Adjusts TSD generator to load ESM interface loader. |
| rosocket/index.js | Converts rosocket server module to ESM exports/imports. |
| rosocket/cli.js | Converts rosocket CLI to ESM imports. |
| rosidl_gen/templates/message-template.cjs | Adjusts generated-message template for ESM/CJS boundary (typed array + distro logic). |
| rosidl_gen/primitive_types.cjs | Adjusts primitive types helper to load native addon under ESM migration. |
| rosidl_gen/index.cjs | Adds generated/ scope CommonJS marker under ESM package root. |
| rosidl_gen/idl_generator.cjs | Adjusts generator’s distro import for ESM library. |
| rosidl_gen/deallocator.cjs | Adjusts deallocator helper to load native addon under ESM migration. |
| package.json | Sets "type": "module" for ESM-first authoring and keeps exports map. |
| lib/wait_for_message.js | Converts waitForMessage implementation to ESM default export. |
| lib/validator.js | Converts validator to ESM default export + adds named export for direct import use. |
| lib/utils.js | Converts utils module to ESM named exports (with maintained aliases). |
| lib/type_description_service.js | Converts type description service implementation to ESM. |
| lib/timer.js | Converts timer implementation to ESM default export. |
| lib/time.js | Converts time implementation to ESM default export. |
| lib/time_source.js | Converts time source implementation to ESM default export. |
| lib/subscription.js | Converts subscription implementation to ESM and updates debug import style. |
| lib/service.js | Converts service implementation to ESM and updates debug import style. |
| lib/service_introspection.js | Converts service introspection constants to ESM default export. |
| lib/serialization.js | Converts serialization helper to ESM and re-exports serialize/deserialize helpers. |
| lib/runtime/transports/ws.js | Converts WS transport to ESM exports/imports. |
| lib/runtime/transports/http.js | Converts HTTP transport to ESM exports/imports. |
| lib/runtime/transport_adapter.js | Converts transport adapter base class to ESM export. |
| lib/runtime/index.js | Converts runtime public surface to ESM named exports. |
| lib/runtime/dispatcher.js | Converts dispatcher to ESM and updates debug import style. |
| lib/runtime/connection.js | Converts connection to ESM (EventEmitter default import). |
| lib/runtime/cli-config.js | Converts CLI config utilities to ESM named exports. |
| lib/runtime/capability_registry.js | Converts capability registry to ESM export. |
| lib/rmw.js | Converts RMW helpers to ESM default export. |
| lib/rate.js | Converts rate module to ESM named exports. |
| lib/qos.js | Converts QoS implementation to ESM default export. |
| lib/qos_overriding_options.js | Converts QoS overriding options to ESM named exports. |
| lib/publisher.js | Converts publisher implementation to ESM and updates debug import style. |
| lib/prebuilds.js | Converts prebuild helper utilities to ESM named exports. |
| lib/parameter.js | Converts parameter module to ESM named exports. |
| lib/parameter_watcher.js | Converts parameter watcher to ESM and updates imports to explicit .js. |
| lib/parameter_service.js | Converts parameter service to ESM default export. |
| lib/parameter_event_handler.js | Converts parameter event handler to ESM default export + named handle exports. |
| lib/parameter_client.js | Converts parameter client to ESM and updates debug import style. |
| lib/observable_subscription.js | Converts observable subscription to ESM default export. |
| lib/node.js | Converts Node implementation to ESM default export and adjusts imports. |
| lib/node_options.js | Converts NodeOptions to ESM default export. |
| lib/native_loader.js | Converts native addon loader to ESM and uses createRequire for runtime loading needs. |
| lib/message_validation.js | Converts message validation helpers to ESM named exports. |
| lib/message_serialization.js | Converts message serialization helpers to ESM named exports. |
| lib/message_introspector.js | Converts message introspector to ESM default export. |
| lib/message_info.js | Converts message info class to ESM default export. |
| lib/logging.js | Converts logging implementation to ESM default export. |
| lib/logging_service.js | Converts logging service to ESM default export. |
| lib/lifecycle.js | Converts lifecycle namespace to ESM default export. |
| lib/lifecycle_publisher.js | Converts lifecycle publisher to ESM default export. |
| lib/interface_loader.js | Converts interface loader to ESM default export and keeps runtime require via createRequire. |
| lib/guard_condition.js | Converts guard condition to ESM default export. |
| lib/event_handler.js | Converts event handler types/helpers to ESM named exports. |
| lib/errors.js | Converts errors module to ESM named exports. |
| lib/entity.js | Converts entity base class to ESM default export. |
| lib/duration.js | Converts duration to ESM default export. |
| lib/distro.js | Converts distro utilities to ESM default export. |
| lib/context.js | Converts context to ESM default export. |
| lib/clock.js | Converts clock module to ESM named exports. |
| lib/clock_type.js | Converts clock type enum to ESM default export. |
| lib/clock_event.js | Converts clock event to ESM default export. |
| lib/clock_change.js | Converts clock change enum to ESM default export. |
| lib/client.js | Converts client implementation to ESM default export. |
| lib/action/uuid.js | Converts action UUID implementation to ESM default export. |
| lib/action/server.js | Converts action server implementation to ESM default export. |
| lib/action/server_goal_handle.js | Converts server goal handle to ESM default export. |
| lib/action/response.js | Converts action response constants to ESM named exports. |
| lib/action/interfaces.js | Converts action interface helper to ESM default export. |
| lib/action/graph.js | Converts action graph helpers to ESM named exports. |
| lib/action/deferred.js | Converts deferred helper to ESM default export. |
| lib/action/client.js | Converts action client implementation to ESM default export. |
| lib/action/client_goal_handle.js | Converts client goal handle to ESM default export. |
| index.js | Converts package entrypoint to ESM default export and updates circular import strategy. |
| eslint.config.mjs | Splits eslint config between ESM and CJS file globs and expands ignores. |
| bin/rclnodejs-web.js | Converts rclnodejs-web launcher to ESM and uses deferred dynamic imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+15
to
+16
| // lib/distro.js is an ES module; require() returns its namespace, so unwrap .default. | ||
| const DistroUtils = require('../lib/distro').default; |
Comment on lines
+6
to
+10
| // native_loader.js is an ES module (default export). When required from CJS, | ||
| // Node returns the module namespace, so unwrap the default export. | ||
| const nativeLoader = require('../../../lib/native_loader.js'); | ||
| const addon = | ||
| nativeLoader && nativeLoader.default ? nativeLoader.default : nativeLoader; |
Comment on lines
17
to
20
| const ref = require('../third_party/ref-napi'); | ||
| const StructType = require('@rclnodejs/ref-struct-di')(ref); | ||
| const rclnodejs = require('../lib/native_loader.js'); | ||
| const rclnodejs = require('../lib/native_loader.js').default; | ||
|
|
Comment on lines
+17
to
18
| const rclnodejs = require('../lib/native_loader.js').default; | ||
|
|
Comment on lines
271
to
304
| @@ -299,7 +300,7 @@ function generateMessage(data) { | |||
|
|
|||
| 'use strict'; | |||
|
|
|||
| ${willUseTypedArray ? "const rclnodejs = require('../../lib/native_loader.js');" : ''} | |||
| ${willUseTypedArray ? "const rclnodejs = require('../../lib/native_loader.js').default;" : ''} | |||
| const ref = require('../../third_party/ref-napi'); | |||
Comment on lines
+31
to
33
| // lib/interface_loader.js is an ES module; require() returns its namespace, so unwrap .default. | ||
| const loader = require('../lib/interface_loader.js').default; | ||
| const pkgFilters = require('../rosidl_gen/filter.cjs'); |
| import rclnodejs from '../index.js'; | ||
| import { createRuntime, WebSocketTransport } from '../lib/runtime/index.js'; | ||
|
|
||
| // `web/` is ESM; await import() lets us pull it in from this CJS file. |
Comment on lines
17
to
23
| const fse = require('../lib/utils.js'); | ||
| const path = require('path'); | ||
| const parser = require('../rosidl_parser/rosidl_parser.cjs'); | ||
| const actionMsgs = require('./action_msgs.cjs'); | ||
| const DistroUtils = require('../lib/distro.js'); | ||
| // lib/distro.js is an ES module; require() returns its namespace, so unwrap .default. | ||
| const DistroUtils = require('../lib/distro.js').default; | ||
| const generateMessage = require('./templates/message-template.cjs'); |
Comment on lines
+45
to
+49
| mocha.loadFilesAsync().then(() => { | ||
| mocha.run(function (failures) { | ||
| process.exitCode = failures ? 1 : 0; | ||
| process.exit(process.exitCode); | ||
| }); |
|
coverage: 91.22% (+5.7%) from 85.523% — minggangw:esm-migration-phase2 into RobotWebTools:develop |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 182 out of 183 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (3)
package.json:12
- Setting "type": "module" makes the package entrypoint (index.js) ESM, but the package doesn't provide a CJS "require" export (or a CJS bundle) yet. As a result,
require('rclnodejs')(and internal .cjs scripts/tests that still userequire('../lib/*.js')) will throwERR_REQUIRE_ESMunder Node 20, breaking backward compatibility and even the current test runner setup.
scripts/ros_distro.cjs:20 - This CommonJS script attempts to load an ES module via require(). On Node 20,
require('../lib/distro')will throwERR_REQUIRE_ESMregardless of unwrapping.default. Useimport()from CJS (or convert the script to ESM) so it can run under the package-wide "type": "module" setting.
test/electron/test_usability.cjs:6 - This CJS file still uses
require('../../index.js'), but index.js is now an ES module because the package is "type": "module". On Node 20 that will throwERR_REQUIRE_ESMbefore the.defaultunwrap runs. Switch toimport()inside the Electron ready handler.
Comment on lines
+11
to
+12
| process.env.NODE_ENV = 'test'; | ||
| const nativeLoader = (await import('../lib/native_loader.js?env=test')).default; |
Comment on lines
+100
to
105
| // Re-evaluate native_loader via a cache-busting dynamic import to trigger | ||
| // loadNativeAddon with RCLNODEJS_FORCE_BUILD. execSync is stubbed so no | ||
| // real rebuild (which deletes generated/) occurs. | ||
| try { | ||
| require('../lib/native_loader.js'); | ||
| await import('../lib/native_loader.js?forcebuild=1'); | ||
| } catch (e) { |
Comment on lines
+110
to
+118
| // index.js gates generateAll() behind a module-level _rosVersionChecked | ||
| // flag that is already true once any earlier test has initialized. ESM | ||
| // modules are singletons and cannot be reloaded via require.cache, so use | ||
| // a cache-busting dynamic import to obtain a fresh index.js instance with | ||
| // _rosVersionChecked === false. Its lib/* imports resolve to the existing | ||
| // singletons (including the same generator CJS module the stub targets). | ||
| const freshRclnodejs = ( | ||
| await import(`../index.js?rollbacktest=${Date.now()}`) | ||
| ).default; |
Comment on lines
+6
to
+10
| // native_loader.js is an ES module (default export). When required from CJS, | ||
| // Node returns the module namespace, so unwrap the default export. | ||
| const nativeLoader = require('../../../lib/native_loader.js'); | ||
| const addon = | ||
| nativeLoader && nativeLoader.default ? nativeLoader.default : nativeLoader; |
Comment on lines
271
to
304
| @@ -299,7 +300,7 @@ function generateMessage(data) { | |||
|
|
|||
| 'use strict'; | |||
|
|
|||
| ${willUseTypedArray ? "const rclnodejs = require('../../lib/native_loader.js');" : ''} | |||
| ${willUseTypedArray ? "const rclnodejs = require('../../lib/native_loader.js').default;" : ''} | |||
| const ref = require('../../third_party/ref-napi'); | |||
Comment on lines
17
to
20
| const ref = require('../third_party/ref-napi'); | ||
| const StructType = require('@rclnodejs/ref-struct-di')(ref); | ||
| const rclnodejs = require('../lib/native_loader.js'); | ||
| const rclnodejs = require('../lib/native_loader.js').default; | ||
|
|
Comment on lines
28
to
33
| const os = require('os'); | ||
| const path = require('path'); | ||
| const fs = require('fs'); | ||
| const loader = require('../lib/interface_loader.js'); | ||
| // lib/interface_loader.js is an ES module; require() returns its namespace, so unwrap .default. | ||
| const loader = require('../lib/interface_loader.js').default; | ||
| const pkgFilters = require('../rosidl_gen/filter.cjs'); |
Comment on lines
17
to
23
| const fse = require('../lib/utils.js'); | ||
| const path = require('path'); | ||
| const parser = require('../rosidl_parser/rosidl_parser.cjs'); | ||
| const actionMsgs = require('./action_msgs.cjs'); | ||
| const DistroUtils = require('../lib/distro.js'); | ||
| // lib/distro.js is an ES module; require() returns its namespace, so unwrap .default. | ||
| const DistroUtils = require('../lib/distro.js').default; | ||
| const generateMessage = require('./templates/message-template.cjs'); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 188 out of 189 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
package.json:12
- With
"type": "module"and anexportsmap that only provides an ESM entry ("default": "./index.js"),require('rclnodejs')will no longer behave like it did in CommonJS (likelyERR_REQUIRE_ESMon older supported Node versions, or returning a namespace object that forces consumers to access.default). This conflicts with the Issue #1358 goal of maintaining backward compatibility for CJS consumers. If CJS support is required in this phase, add conditional exports (requirevsimport) and/or a bundled CJS build output.
Comment on lines
+20
to
+22
| import { NativeError } from './errors.js'; | ||
| import bindings from 'bindings'; | ||
| import createDebug from 'debug'; |
Comment on lines
+15
to
+16
| // lib/distro.js is an ES module; require() returns its namespace, so unwrap .default. | ||
| const DistroUtils = require('../lib/distro').default; |
Comment on lines
+6
to
+10
| // native_loader.js is an ES module (default export). When required from CJS, | ||
| // Node returns the module namespace, so unwrap the default export. | ||
| const nativeLoader = require('../../../lib/native_loader.js'); | ||
| const addon = | ||
| nativeLoader && nativeLoader.default ? nativeLoader.default : nativeLoader; |
Comment on lines
+45
to
50
| mocha.loadFilesAsync().then(() => { | ||
| mocha.run(function (failures) { | ||
| process.exitCode = failures ? 1 : 0; | ||
| process.exit(process.exitCode); | ||
| }); | ||
| }); |
Comment on lines
+19
to
+23
| import { | ||
| ParameterType, | ||
| Parameter, | ||
| ParameterDescriptor, | ||
| } = require('../lib/parameter.js'); | ||
| } from '../lib/parameter.js'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview: 189 files changed, +957 / −1162. Converts
lib/,index.js,bin/,rosocket/and the full test suite to native ES modules; keeps build/codegen tooling as.cjs; updates CI to ROS 2 Lyrical (Ubuntu 26.04); and fixes a native-addon debug-build compile error.Core ESM conversion
index.js— converted to ESM entrypoint (import/export), module wiring reworked.lib/**— all library modules converted fromrequire/module.exportstoimport/export(node, client, service, publisher, subscription, action/, clock, parameter*, logging*, runtime/, serialization, time, timer, qos*, utils, validator, native_loader, etc.).bin/rclnodejs-web.js,rosocket/index.js,rosocket/cli.js— converted to ESM.ESM/CJS boundary fixes (from Copilot review)
lib/native_loader.js— load the CommonJSbindingshelper viacreateRequire'srequire('bindings')instead ofimport bindings from 'bindings', preserving CJS caller context for addon resolution.lib/type_description_service.js— corrected sibling import from'../lib/parameter.js'to'./parameter.js'.CommonJS files kept as
.cjsrosidl_gen/*.cjs(deallocator, idl_generator, index, primitive_types, templates/message-template),rostsd_gen/index.cjs,scripts/ros_distro.cjs,scripts/run_test.cjs— build/codegen tooling stays CJS (loads ESM helpers via supportedrequire(esm)/.default).third_party/ref-napi/—lib/ref.jsandpackage.jsontweaks for module resolution.Tooling / config
package.json—"type": "module"and script adjustments.eslint.config.mjs— ESM lint rules..c8rc.jsonadded,.nycrc.ymlremoved — switch coverage from nyc to c8.CI
.github/workflows/linux-x64-asan-test.yml,.github/workflows/linux-x64-build-and-test.yml— migrated from ROS 2 Jazzy to Lyrical (Ubuntu 26.04)..github/actions/setup-ros2-apt/action.yml(new) — composite action to install Lyrical from apt (setup-ros doesn't support it yet).Native addon fix
src/macros.h— replaced removed internal rcutils macros (RCUTILS_LOG_COND_NAMED/RCUTILS_LOG_CONDITION_EMPTY) inRCLNODEJS_DEBUGwith the stable publicRCUTILS_LOG_DEBUG_NAMED, fixing the ASan debug-build compile failure under Lyrical.Tests
test/test-*.jsconverted to ESM;tutorials/.../test_usability.jsrenamed to.cjs;test/utils.jsupdated.Fix: #1358