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

💄 linter error: Unexpected aliasing of 'this' to local variable #102

Closed
jdhoffa opened this issue Dec 2, 2024 · 1 comment · Fixed by #115
Closed

💄 linter error: Unexpected aliasing of 'this' to local variable #102

jdhoffa opened this issue Dec 2, 2024 · 1 comment · Fixed by #115
Assignees
Labels

Comments

@jdhoffa
Copy link
Member

jdhoffa commented Dec 2, 2024

I have silenced the errors for now, but these lines are causing complaints:

textLabels.each(function () {
thisLabel = this;
thisLabelDoc = d3.select(thisLabel);
y1 = thisLabelDoc.attr('y');
textLabels.each(function () {
thatLabel = this;
if (thisLabel == thatLabel) return;

We should look into a solution (rather than just disabling the linter)

@cjyetman
Copy link
Member

cjyetman commented Dec 3, 2024

starting form here:

textLabels.each(function () {
thisLabel = this; // eslint-disable-line
thisLabelDoc = d3.select(thisLabel);
y1 = thisLabelDoc.attr('y');
textLabels.each(function () {
thatLabel = this; // eslint-disable-line
if (thisLabel == thatLabel) return;
thatLabelDoc = d3.select(thatLabel);
if (thisLabelDoc.attr('text-anchor') != thatLabelDoc.attr('text-anchor')) return;
y2 = thatLabelDoc.attr('y');
deltaY = y1 - y2;
if (Math.abs(deltaY) > minLabelSpacing) return;
again = true;
sign = deltaY > 0 ? 1 : -1;
adjustment = sign * alpha;
thisLabelDoc.attr('y', +y1 + adjustment);
thatLabelDoc.attr('y', +y2 - adjustment);
});
});

We can remove the this alias from the second level deep .each() like this (with a little bit of restructuring for better readability)

 textLabels.each(function () {
 	thisLabelDoc = d3.select(this);
 	y1 = thisLabelDoc.attr('y');

        thisLabel = this; // eslint-disable-line

 	textLabels.each(function () {
                thatLabelDoc = d3.select(this);
                y2 = thatLabelDoc.attr('y');

 		if (thisLabel == this) return;
 		if (thisLabelDoc.attr('text-anchor') != thatLabelDoc.attr('text-anchor')) return;

 		deltaY = y1 - y2;
 		if (Math.abs(deltaY) > minLabelSpacing) return;

 		again = true;
 		sign = deltaY > 0 ? 1 : -1;
 		adjustment = sign * alpha;
 		thisLabelDoc.attr('y', +y1 + adjustment);
 		thatLabelDoc.attr('y', +y2 - adjustment);
 	});
 });

I believe the intent of if (thisLabel == this) return; is to prevent the 2nd level deep function from comparing a label to itself, but it's harder to get rid of the 1st level deep this alias because you need it to compare to this in the 2nd level deep .each() function. Maybe we could work around that by comparing thisLabelDoc to thatLabelDoc instead with something like...

 textLabels.each(function () {
 	thisLabelDoc = d3.select(this);
 	y1 = thisLabelDoc.attr('y');

 	textLabels.each(function () {
                thatLabelDoc = d3.select(this);
                y2 = thatLabelDoc.attr('y');

 		if (thisLabelDoc == thatLabelDoc) return;
 		if (thisLabelDoc.attr('text-anchor') != thatLabelDoc.attr('text-anchor')) return;

 		deltaY = y1 - y2;
 		if (Math.abs(deltaY) > minLabelSpacing) return;

 		again = true;
 		sign = deltaY > 0 ? 1 : -1;
 		adjustment = sign * alpha;
 		thisLabelDoc.attr('y', +y1 + adjustment);
 		thatLabelDoc.attr('y', +y2 - adjustment);
 	});
 });

not exactly sure what an element of textLabel looks like though, especially since this .each() function is a jQuery function maybe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants