Skip to content
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.

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

chore: upgrade eventemitter to v5.0.2 #7709

Merged
merged 15 commits into from
May 14, 2024
Merged
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"eslint-plugin-unicorn": "49.0.0",
"eslint-plugin-vue": "9.22.0",
"eslint-plugin-you-dont-need-lodash-underscore": "6.13.0",
"eventemitter3": "1.2.0",
"eventemitter3": "5.0.1",
"file-saver": "2.0.5",
"flatbush": "4.2.0",
"git-rev-sync": "3.0.2",
Expand Down
1 change: 0 additions & 1 deletion src/MCT.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ import Browse from './ui/router/Browse.js';
export class MCT extends EventEmitter {
constructor() {
super();
EventEmitter.call(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Whaaaat were we even doing here? Suspect this was a hangover from prototypical inheritance that was left in by mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, exactly.


this.buildInfo = {
version: __OPENMCT_VERSION__,
Expand Down
2 changes: 0 additions & 2 deletions src/plugins/LADTable/components/LadRow.vue
Original file line number Diff line number Diff line change
Expand Up @@ -237,13 +237,11 @@ export default {
}

this.previewAction = new PreviewAction(this.openmct);
this.previewAction.on('isVisible', this.togglePreviewState);
Copy link
Contributor Author

@ozyx ozyx May 2, 2024

Choose a reason for hiding this comment

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

this.togglePreviewState was undefined, and this wasn't doing anything except making our unit tests fail. It seems that the new version throws an exception if you try to unregister a listener that doesn't exist, or register a listener without context.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you're right. I wonder if our preview action changed at some point as it doesn't seem to have much benefit to emitting this 'isVisible' event at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that the new version throws an exception if you try to unregister a listener that doesn't exist, or register a listener without context.

From v3.0.0 onwards, EventEmitter3 throws an error if you try to register a listener with a callback that is not a function

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, this is being used for search results here:
https://github.com/nasa/openmct/blob/master/src/ui/layout/search/SearchResultsDropDown.vue#L48
It was likely a copy-paste error where the this.togglePreviewState method was not added. It is also missing in AnnotationSearchResult.vue

You can see an implementation here:
https://github.com/nasa/openmct/blob/master/src/ui/layout/RecentObjectsListItem.vue#L116

},
unmounted() {
this.openmct.time.off('timeSystem', this.updateTimeSystem);
this.telemetryCollection.off('add', this.setLatestValues);
this.telemetryCollection.off('clear', this.resetValues);
this.previewAction.off('isVisible', this.togglePreviewState);

this.telemetryCollection.destroy();
},
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/imagery/lib/eventHelpers.js
ozyx marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const helperFunctions = {
} else if (object.addEventListener) {
object.addEventListener(event, listener._cb);
} else {
object.on(event, listener._cb);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, so how was this working before? The context would have been undefined?

object.on(event, listener._cb, listener.context);
}

this._listeningTo.push(listener);
Expand Down Expand Up @@ -78,7 +78,7 @@ const helperFunctions = {
} else if (listener.object.removeEventListener) {
listener.object.removeEventListener(listener.event, listener._cb);
} else {
listener.object.off(listener.event, listener._cb);
listener.object.off(listener.event, listener._cb, listener.context);
}

return listener;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/plot/chart/MctChart.vue
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ export default {
this.canvas = mainCanvas;
this.overlay = overlayCanvas;
this.drawAPI = DrawLoader.getDrawAPI(mainCanvas, overlayCanvas);
if (this.drawAPI) {
if (this.drawAPI && this.drawAPI.on) {
ozyx marked this conversation as resolved.
Show resolved Hide resolved
this.listenTo(this.drawAPI, 'error', this.fallbackToCanvas, this);
}

Expand Down
179 changes: 85 additions & 94 deletions src/plugins/plot/draw/Draw2D.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,113 +39,104 @@ import { MARKER_SHAPES } from './MarkerShapes.js';
* @param {CanvasElement} canvas the canvas object to render upon
* @throws {Error} an error is thrown if Canvas's 2D API is unavailable
*/
function Draw2D(canvas) {
this.canvas = canvas;
this.c2d = canvas.getContext('2d');
this.width = canvas.width;
this.height = canvas.height;
this.dimensions = [this.width, this.height];
this.origin = [0, 0];

if (!this.c2d) {
throw new Error('Canvas 2d API unavailable.');
class Draw2D extends EventEmitter {
constructor(canvas) {
super();
eventHelpers.extend(this);
this.canvas = canvas;
this.c2d = canvas.getContext('2d');
this.width = canvas.width;
this.height = canvas.height;
this.dimensions = [this.width, this.height];
this.origin = [0, 0];

if (!this.c2d) {
throw new Error('Canvas 2d API unavailable.');
}
}
}

Object.assign(Draw2D.prototype, EventEmitter.prototype);
eventHelpers.extend(Draw2D.prototype);

// Convert from logical to physical x coordinates
Draw2D.prototype.x = function (v) {
return ((v - this.origin[0]) / this.dimensions[0]) * this.width;
};

// Convert from logical to physical y coordinates
Draw2D.prototype.y = function (v) {
return this.height - ((v - this.origin[1]) / this.dimensions[1]) * this.height;
};

// Set the color to be used for drawing operations
Draw2D.prototype.setColor = function (color) {
const mappedColor = color
.map(function (c, i) {
return i < 3 ? Math.floor(c * 255) : c;
})
.join(',');
this.c2d.strokeStyle = 'rgba(' + mappedColor + ')';
this.c2d.fillStyle = 'rgba(' + mappedColor + ')';
};

Draw2D.prototype.clear = function () {
this.width = this.canvas.width = this.canvas.offsetWidth;
this.height = this.canvas.height = this.canvas.offsetHeight;
this.c2d.clearRect(0, 0, this.width, this.height);
};

Draw2D.prototype.setDimensions = function (newDimensions, newOrigin) {
this.dimensions = newDimensions;
this.origin = newOrigin;
};

Draw2D.prototype.drawLine = function (buf, color, points) {
let i;

this.setColor(color);

// Configure context to draw two-pixel-thick lines
this.c2d.lineWidth = 1;

// Start a new path...
if (buf.length > 1) {
this.c2d.beginPath();
this.c2d.moveTo(this.x(buf[0]), this.y(buf[1]));
// Convert from logical to physical x coordinates
x(v) {
return ((v - this.origin[0]) / this.dimensions[0]) * this.width;
}

// ...and add points to it...
for (i = 2; i < points * 2; i = i + 2) {
this.c2d.lineTo(this.x(buf[i]), this.y(buf[i + 1]));
// Convert from logical to physical y coordinates
y(v) {
return this.height - ((v - this.origin[1]) / this.dimensions[1]) * this.height;
}
// Set the color to be used for drawing operations
setColor(color) {
const mappedColor = color
.map(function (c, i) {
return i < 3 ? Math.floor(c * 255) : c;
})
.join(',');
this.c2d.strokeStyle = 'rgba(' + mappedColor + ')';
this.c2d.fillStyle = 'rgba(' + mappedColor + ')';
}
clear() {
this.width = this.canvas.width = this.canvas.offsetWidth;
this.height = this.canvas.height = this.canvas.offsetHeight;
this.c2d.clearRect(0, 0, this.width, this.height);
}
setDimensions(newDimensions, newOrigin) {
this.dimensions = newDimensions;
this.origin = newOrigin;
}
drawLine(buf, color, points) {
let i;

// ...before finally drawing it.
this.c2d.stroke();
};

Draw2D.prototype.drawSquare = function (min, max, color) {
const x1 = this.x(min[0]);
const y1 = this.y(min[1]);
const w = this.x(max[0]) - x1;
const h = this.y(max[1]) - y1;
this.setColor(color);

this.setColor(color);
this.c2d.fillRect(x1, y1, w, h);
};
// Configure context to draw two-pixel-thick lines
this.c2d.lineWidth = 1;

Draw2D.prototype.drawPoints = function (buf, color, points, pointSize, shape) {
const drawC2DShape = MARKER_SHAPES[shape].drawC2D.bind(this);
// Start a new path...
if (buf.length > 1) {
this.c2d.beginPath();
this.c2d.moveTo(this.x(buf[0]), this.y(buf[1]));
}

this.setColor(color);
// ...and add points to it...
for (i = 2; i < points * 2; i = i + 2) {
this.c2d.lineTo(this.x(buf[i]), this.y(buf[i + 1]));
}

for (let i = 0; i < points; i++) {
drawC2DShape(this.x(buf[i * 2]), this.y(buf[i * 2 + 1]), pointSize);
// ...before finally drawing it.
this.c2d.stroke();
}
drawSquare(min, max, color) {
const x1 = this.x(min[0]);
const y1 = this.y(min[1]);
const w = this.x(max[0]) - x1;
const h = this.y(max[1]) - y1;

this.setColor(color);
this.c2d.fillRect(x1, y1, w, h);
}
};
drawPoints(buf, color, points, pointSize, shape) {
const drawC2DShape = MARKER_SHAPES[shape].drawC2D.bind(this);

Draw2D.prototype.drawLimitPoint = function (x, y, size) {
this.c2d.fillRect(x + size, y, size, size);
this.c2d.fillRect(x, y + size, size, size);
this.c2d.fillRect(x - size, y, size, size);
this.c2d.fillRect(x, y - size, size, size);
};
this.setColor(color);

Draw2D.prototype.drawLimitPoints = function (points, color, pointSize) {
const limitSize = pointSize * 2;
const offset = limitSize / 2;
for (let i = 0; i < points; i++) {
drawC2DShape(this.x(buf[i * 2]), this.y(buf[i * 2 + 1]), pointSize);
}
}
drawLimitPoint(x, y, size) {
this.c2d.fillRect(x + size, y, size, size);
this.c2d.fillRect(x, y + size, size, size);
this.c2d.fillRect(x - size, y, size, size);
this.c2d.fillRect(x, y - size, size, size);
}
drawLimitPoints(points, color, pointSize) {
const limitSize = pointSize * 2;
const offset = limitSize / 2;

this.setColor(color);
this.setColor(color);

for (let i = 0; i < points.length; i++) {
this.drawLimitPoint(this.x(points[i].x) - offset, this.y(points[i].y) - offset, limitSize);
for (let i = 0; i < points.length; i++) {
this.drawLimitPoint(this.x(points[i].x) - offset, this.y(points[i].y) - offset, limitSize);
}
}
};
}

export default Draw2D;
Loading
Loading