Skip to content

Remove compatibility and custom utility code #1479

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
minggangw merged 3 commits into from
Apr 8, 2026
Merged

Remove compatibility and custom utility code #1479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b90b0b8
Remove compatibility and custom utility code
minggangw Apr 7, 2026
f8779bb
Address comments
minggangw Apr 8, 2026
32241ee
Address comments
minggangw Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/action/uuid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class ActionUuid {
this._bytes = bytes;
} else {
// Generate random UUID.
let uuid = randomUUID().replace(/-/g, '');
let uuid = randomUUID().replaceAll('-', '');
this._bytes = Uint8Array.from(Buffer.from(uuid, 'hex'));
}
}
Expand Down
45 changes: 0 additions & 45 deletions lib/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,51 +25,6 @@ const {
const { assertValidMessage } = require('./message_validation.js');
const debug = require('debug')('rclnodejs:client');

Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

Removing the AbortSignal.any() polyfill makes this module rely on a native AbortSignal.any implementation, but the code still calls AbortSignal.any later (e.g., in sendRequestAsync) without a guard. With the current repo metadata/CI still targeting Node 16.x (package.json engines and ARM64 workflow), this will throw at runtime. Either restore a local fallback for AbortSignal.any (only when missing) or update the supported Node.js floor + CI/prebuild targets/docs in the same PR so this file’s runtime requirements are consistent.

Suggested change
if (
typeof AbortSignal !== 'undefined' &&
typeof AbortSignal.any !== 'function'
) {
AbortSignal.any = function any(signals) {
if (!signals || typeof signals[Symbol.iterator] !== 'function') {
throw new TypeError(
"Failed to execute 'any' on 'AbortSignal': parameter 1 is not iterable."
);
}
const signalList = Array.from(signals);
const controller = new AbortController();
const listeners = new Map();
const cleanup = function cleanup() {
for (const signal of signalList) {
const listener = listeners.get(signal);
if (listener) {
signal.removeEventListener('abort', listener);
}
}
listeners.clear();
};
for (const signal of signalList) {
if (signal.aborted) {
controller.abort(signal.reason);
return controller.signal;
}
}
for (const signal of signalList) {
const listener = function listener() {
cleanup();
controller.abort(signal.reason);
};
listeners.set(signal, listener);
signal.addEventListener('abort', listener, { once: true });
}
return controller.signal;
};
}

Copilot uses AI. Check for mistakes.
// Polyfill for AbortSignal.any() for Node.js <= 20.3.0
// AbortSignal.any() was added in Node.js 20.3.0
// See https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/any_static
if (!AbortSignal.any) {
AbortSignal.any = function (signals) {
// Filter out null/undefined values and validate inputs
const validSignals = Array.isArray(signals)
? signals.filter((signal) => signal != null)
: [];

// If no valid signals, return a never-aborting signal
if (validSignals.length === 0) {
return new AbortController().signal;
}

const controller = new AbortController();
const listeners = [];

// Cleanup function to remove all event listeners
const cleanup = () => {
listeners.forEach(({ signal, listener }) => {
signal.removeEventListener('abort', listener);
});
};

for (const signal of validSignals) {
if (signal.aborted) {
cleanup();
controller.abort(signal.reason);
return controller.signal;
}

const listener = () => {
cleanup();
controller.abort(signal.reason);
};

signal.addEventListener('abort', listener);
listeners.push({ signal, listener });
}

return controller.signal;
};
}

/**
* @class - Class representing a Client in ROS
* @hideconstructor
Expand Down
2 changes: 1 addition & 1 deletion lib/interface_loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ let interfaceLoader = {
}

// TODO(Kenny): more checks of the string argument
if (name.indexOf('/') !== -1) {
if (name.includes('/')) {
let [packageName, type, messageName] = name.split('/');
return this.loadInterface(packageName, type, messageName);
}
Expand Down
30 changes: 1 addition & 29 deletions lib/message_introspector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,35 +88,7 @@ class MessageIntrospector {
const instance = new this.#typeClass();
this.#defaultsCache = toPlainObject(instance);
}
return this.#deepClone(this.#defaultsCache);
}

/**
* Deep clone an object.
* @param {any} obj - Object to clone
* @returns {any} Cloned object
* @private
*/
#deepClone(obj) {
if (obj === null || typeof obj !== 'object') {
return obj;
}

if (Array.isArray(obj)) {
return obj.map((item) => this.#deepClone(item));
}

if (ArrayBuffer.isView(obj) && !(obj instanceof DataView)) {
return obj.slice();
}

const cloned = {};
for (const key in obj) {
if (Object.prototype.hasOwnProperty.call(obj, key)) {
cloned[key] = this.#deepClone(obj[key]);
}
}
return cloned;
return structuredClone(this.#defaultsCache);
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

structuredClone is used unconditionally here. If this package still supports Node 16.x (per package.json engines / docs), global structuredClone may be undefined and this getter will throw. Consider feature-detecting structuredClone and falling back to the previous deep-clone logic (or a compatible clone helper), or raise the minimum supported Node.js version and update the repo metadata/CI accordingly.

Copilot uses AI. Check for mistakes.
}
}

Expand Down
4 changes: 2 additions & 2 deletions lib/message_serialization.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function toPlainArrays(obj) {
if (typeof obj === 'object' && obj !== null) {
const result = {};
for (const key in obj) {
if (Object.prototype.hasOwnProperty.call(obj, key)) {
if (Object.hasOwn(obj, key)) {
result[key] = toPlainArrays(obj[key]);
}
}
Expand Down Expand Up @@ -105,7 +105,7 @@ function toJSONSafe(obj) {
if (typeof obj === 'object' && obj !== null) {
const result = {};
for (const key in obj) {
if (Object.prototype.hasOwnProperty.call(obj, key)) {
if (Object.hasOwn(obj, key)) {
result[key] = toJSONSafe(obj[key]);
}
}
Expand Down
8 changes: 4 additions & 4 deletions rosidl_gen/packages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const serviceMsgPath = path.join(generatedRoot, 'srv_msg');

function getPackageName(filePath, amentExecuted) {
if (os.type() === 'Windows_NT') {
filePath = filePath.replace(/\\/g, '/');
filePath = filePath.replaceAll('\\', '/');
}

if (amentExecuted) {
Expand All @@ -41,14 +41,14 @@ function getPackageName(filePath, amentExecuted) {

// If |packageName| equals to the file's extension, e.g. msg/srv, one level
// up directory will be used as the package name.
return packageName === path.parse(filePath).ext.substr(1)
return packageName === path.parse(filePath).ext.substring(1)
? folders.pop()
: packageName;
}

function getSubFolder(filePath, amentExecuted) {
if (os.type() === 'Windows_NT') {
filePath = filePath.replace(/\\/g, '/');
filePath = filePath.replaceAll('\\', '/');
}

if (amentExecuted) {
Expand All @@ -61,7 +61,7 @@ function getSubFolder(filePath, amentExecuted) {
}
// If the |amentExecuted| equals to false, the file's extension will be assigned as
// the name of sub folder.
return path.parse(filePath).ext.substr(1);
return path.parse(filePath).ext.substring(1);
}

function grabInterfaceInfo(filePath, amentExecuted) {
Expand Down
10 changes: 5 additions & 5 deletions rosidl_gen/templates/message-template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,15 @@ const typedArrayType = [
];

function isPrimitivePackage(baseType) {
return primitiveBaseType.indexOf(baseType.type) !== -1;
return primitiveBaseType.includes(baseType.type);
}

function isTypedArrayType(type) {
return typedArrayType.indexOf(type.type.toLowerCase()) !== -1;
return typedArrayType.includes(type.type.toLowerCase());
}

function isBigInt(type) {
return ['int64', 'uint64'].indexOf(type.type.toLowerCase()) !== -1;
return ['int64', 'uint64'].includes(type.type.toLowerCase());
}

function getWrapperNameByType(type) {
Expand Down Expand Up @@ -283,7 +283,7 @@ function generateMessage(data) {
!fieldType.isPrimitiveType ||
fieldType.type === 'string');

if (shouldReq && existedModules.indexOf(requiredModule) === -1) {
if (shouldReq && !existedModules.includes(requiredModule)) {
existedModules.push(requiredModule);
return true;
} else {
Expand Down Expand Up @@ -692,7 +692,7 @@ ${spec.fields

hasMember(name) {
let memberNames = ${extractMemberNames(spec.fields)};
return memberNames.indexOf(name) !== -1;
return memberNames.includes(name);
}
}`;
}
Expand Down
4 changes: 2 additions & 2 deletions scripts/run_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ utils
const testDir = path.join(__dirname, '../test/');
// eslint-disable-next-line
const tests = fs.readdirSync(testDir).filter((file) => {
return file.substr(0, 5) === 'test-';
return file.startsWith('test-');
});

// eslint-disable-next-line
Expand All @@ -37,7 +37,7 @@ utils
let ignoredCases = blocklist[os.type()];

tests.forEach((test) => {
if (ignoredCases.indexOf(test) === -1) {
if (!ignoredCases.includes(test)) {
mocha.addFile(path.join(testDir, test));
}
});
Expand Down
Loading