Skip to content

Commit 79ce8fc

Browse files
authored
Remove compatibility and custom utility code (#1479)
## Changed Files - [lib/client.js](lib/client.js) - Removed the `AbortSignal.any()` polyfill for older Node.js versions. - Net effect: the code now relies on native `AbortSignal.any()` support from the runtime. - [lib/message_introspector.js](lib/message_introspector.js) - Replaced the custom private `#deepClone()` implementation with `structuredClone(this.#defaultsCache)`. - Net effect: simpler default-message cloning logic with less handwritten recursion. - [lib/message_serialization.js](lib/message_serialization.js) - Replaced `Object.prototype.hasOwnProperty.call(obj, key)` with `Object.hasOwn(obj, key)` in two object-walk helpers. - Net effect: minor modernization with no intended behavior change. - [lib/action/uuid.js](lib/action/uuid.js) - Replaced `.replace(/-/g, '')` with `.replaceAll('-', '')`. - [rosidl_gen/packages.js](rosidl_gen/packages.js) - Replaced `.replace(/\\\\/g, '/')` with `.replaceAll('\\', '/')` in two path-normalization helpers. - Replaced deprecated `.substr(1)` with `.substring(1)`. - [rosidl_gen/templates/message-template.js](rosidl_gen/templates/message-template.js) - Replaced `.indexOf(...) !== -1` with `.includes(...)` in `isPrimitivePackage()` and `isTypedArrayType()`. - [scripts/run_test.js](scripts/run_test.js) - Replaced `.substr(0, 5) === 'test-'` with `.startsWith('test-')`. - Replaced `.indexOf(test) === -1` with `!...includes(test)`. ## Overall Impact - Removes legacy compatibility code (`AbortSignal.any()` polyfill, custom `#deepClone()`). - Replaces deprecated APIs (`substr`) and verbose patterns (regex `.replace`, `indexOf`, `hasOwnProperty.call`) with modern equivalents (`substring`, `replaceAll`, `includes`, `startsWith`, `Object.hasOwn`, `structuredClone`). - All replacements are safe, drop-in substitutions available since Node.js 16–20, well within the 20.20.2 minimum. Fix: #1458
1 parent b83aa44 commit 79ce8fc

8 files changed

Lines changed: 16 additions & 89 deletions

File tree

lib/action/uuid.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class ActionUuid {
4040
this._bytes = bytes;
4141
} else {
4242
// Generate random UUID.
43-
let uuid = randomUUID().replace(/-/g, '');
43+
let uuid = randomUUID().replaceAll('-', '');
4444
this._bytes = Uint8Array.from(Buffer.from(uuid, 'hex'));
4545
}
4646
}

lib/client.js

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -25,51 +25,6 @@ const {
2525
const { assertValidMessage } = require('./message_validation.js');
2626
const debug = require('debug')('rclnodejs:client');
2727

28-
// Polyfill for AbortSignal.any() for Node.js <= 20.3.0
29-
// AbortSignal.any() was added in Node.js 20.3.0
30-
// See https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/any_static
31-
if (!AbortSignal.any) {
32-
AbortSignal.any = function (signals) {
33-
// Filter out null/undefined values and validate inputs
34-
const validSignals = Array.isArray(signals)
35-
? signals.filter((signal) => signal != null)
36-
: [];
37-
38-
// If no valid signals, return a never-aborting signal
39-
if (validSignals.length === 0) {
40-
return new AbortController().signal;
41-
}
42-
43-
const controller = new AbortController();
44-
const listeners = [];
45-
46-
// Cleanup function to remove all event listeners
47-
const cleanup = () => {
48-
listeners.forEach(({ signal, listener }) => {
49-
signal.removeEventListener('abort', listener);
50-
});
51-
};
52-
53-
for (const signal of validSignals) {
54-
if (signal.aborted) {
55-
cleanup();
56-
controller.abort(signal.reason);
57-
return controller.signal;
58-
}
59-
60-
const listener = () => {
61-
cleanup();
62-
controller.abort(signal.reason);
63-
};
64-
65-
signal.addEventListener('abort', listener);
66-
listeners.push({ signal, listener });
67-
}
68-
69-
return controller.signal;
70-
};
71-
}
72-
7328
/**
7429
* @class - Class representing a Client in ROS
7530
* @hideconstructor

lib/interface_loader.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ let interfaceLoader = {
5757
}
5858

5959
// TODO(Kenny): more checks of the string argument
60-
if (name.indexOf('/') !== -1) {
60+
if (name.includes('/')) {
6161
let [packageName, type, messageName] = name.split('/');
6262
return this.loadInterface(packageName, type, messageName);
6363
}

lib/message_introspector.js

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -88,35 +88,7 @@ class MessageIntrospector {
8888
const instance = new this.#typeClass();
8989
this.#defaultsCache = toPlainObject(instance);
9090
}
91-
return this.#deepClone(this.#defaultsCache);
92-
}
93-
94-
/**
95-
* Deep clone an object.
96-
* @param {any} obj - Object to clone
97-
* @returns {any} Cloned object
98-
* @private
99-
*/
100-
#deepClone(obj) {
101-
if (obj === null || typeof obj !== 'object') {
102-
return obj;
103-
}
104-
105-
if (Array.isArray(obj)) {
106-
return obj.map((item) => this.#deepClone(item));
107-
}
108-
109-
if (ArrayBuffer.isView(obj) && !(obj instanceof DataView)) {
110-
return obj.slice();
111-
}
112-
113-
const cloned = {};
114-
for (const key in obj) {
115-
if (Object.prototype.hasOwnProperty.call(obj, key)) {
116-
cloned[key] = this.#deepClone(obj[key]);
117-
}
118-
}
119-
return cloned;
91+
return structuredClone(this.#defaultsCache);
12092
}
12193
}
12294

lib/message_serialization.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ function toPlainArrays(obj) {
6262
if (typeof obj === 'object' && obj !== null) {
6363
const result = {};
6464
for (const key in obj) {
65-
if (Object.prototype.hasOwnProperty.call(obj, key)) {
65+
if (Object.hasOwn(obj, key)) {
6666
result[key] = toPlainArrays(obj[key]);
6767
}
6868
}
@@ -105,7 +105,7 @@ function toJSONSafe(obj) {
105105
if (typeof obj === 'object' && obj !== null) {
106106
const result = {};
107107
for (const key in obj) {
108-
if (Object.prototype.hasOwnProperty.call(obj, key)) {
108+
if (Object.hasOwn(obj, key)) {
109109
result[key] = toJSONSafe(obj[key]);
110110
}
111111
}

rosidl_gen/packages.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const serviceMsgPath = path.join(generatedRoot, 'srv_msg');
2929

3030
function getPackageName(filePath, amentExecuted) {
3131
if (os.type() === 'Windows_NT') {
32-
filePath = filePath.replace(/\\/g, '/');
32+
filePath = filePath.replaceAll('\\', '/');
3333
}
3434

3535
if (amentExecuted) {
@@ -41,14 +41,14 @@ function getPackageName(filePath, amentExecuted) {
4141

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

4949
function getSubFolder(filePath, amentExecuted) {
5050
if (os.type() === 'Windows_NT') {
51-
filePath = filePath.replace(/\\/g, '/');
51+
filePath = filePath.replaceAll('\\', '/');
5252
}
5353

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

6767
function grabInterfaceInfo(filePath, amentExecuted) {

rosidl_gen/templates/message-template.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,15 +156,15 @@ const typedArrayType = [
156156
];
157157

158158
function isPrimitivePackage(baseType) {
159-
return primitiveBaseType.indexOf(baseType.type) !== -1;
159+
return primitiveBaseType.includes(baseType.type);
160160
}
161161

162162
function isTypedArrayType(type) {
163-
return typedArrayType.indexOf(type.type.toLowerCase()) !== -1;
163+
return typedArrayType.includes(type.type.toLowerCase());
164164
}
165165

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

170170
function getWrapperNameByType(type) {
@@ -283,7 +283,7 @@ function generateMessage(data) {
283283
!fieldType.isPrimitiveType ||
284284
fieldType.type === 'string');
285285

286-
if (shouldReq && existedModules.indexOf(requiredModule) === -1) {
286+
if (shouldReq && !existedModules.includes(requiredModule)) {
287287
existedModules.push(requiredModule);
288288
return true;
289289
} else {
@@ -692,7 +692,7 @@ ${spec.fields
692692
693693
hasMember(name) {
694694
let memberNames = ${extractMemberNames(spec.fields)};
695-
return memberNames.indexOf(name) !== -1;
695+
return memberNames.includes(name);
696696
}
697697
}`;
698698
}

scripts/run_test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ utils
2727
const testDir = path.join(__dirname, '../test/');
2828
// eslint-disable-next-line
2929
const tests = fs.readdirSync(testDir).filter((file) => {
30-
return file.substr(0, 5) === 'test-';
30+
return file.startsWith('test-');
3131
});
3232

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

3939
tests.forEach((test) => {
40-
if (ignoredCases.indexOf(test) === -1) {
40+
if (!ignoredCases.includes(test)) {
4141
mocha.addFile(path.join(testDir, test));
4242
}
4343
});

0 commit comments

Comments
 (0)