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.

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

Jump to bottom

add Standard #8

Merged
merged 2 commits into from
Oct 1, 2023
Merged

add Standard #8

merged 2 commits into from
Oct 1, 2023

Conversation

mixmix
Copy link
Member

@mixmix mixmix commented Sep 28, 2023 •
edited
Loading

esbuild surfaced some bugs, which got me wondering if standard would have.

Turns out it did... these are the important ones (but there were also a few unused vars to tidy):

/eventually.js:6:7: '_listener' is not defined. (no-undef)
/eventually.js:8:7: '_listener' is not defined. (no-undef)
/eventually.js:9:7: Unnecessary return statement. (no-useless-return)
/filter.js:5:7: 'listeners' is assigned a value but never used. (no-unused-vars)
/filter.js:6:7: 'rm' is assigned a value but never used. (no-unused-vars)
/index.js:66:34: 'i' is constant. (no-const-assign)
/single.js:10:7: 'listener' is not defined. (no-undef)
/single.js:12:10: 'listener' is not defined. (no-undef)
/single.js:12:30: 'listener' is not defined. (no-undef)
/single.js:17:22: '_value' is not defined. (no-undef)
/single.js:18:10: 'listener' is not defined. (no-undef)
/single.js:18:20: 'listener' is not defined. (no-undef)
/single.js:18:37: '_value' is not defined. (no-undef)

@mixmix mixmix changed the base branch from master to once.remove September 28, 2023 22:25
@socket-security
Copy link

socket-security bot commented Sep 28, 2023 •
edited
Loading

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
standard 17.1.0 None +144 15.8 MB voxpelli

mixmix
mixmix commented Sep 28, 2023
View reviewed changes
single.js
observer.set = function (value) {
if(filter(value, _value)) {
if(listener) listener(value = _value)
observer.set = function (_value) {
Copy link
Member Author

Choose a reason for hiding this comment

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

🔥

mixmix
mixmix commented Sep 28, 2023
View reviewed changes
index.js
value = nextValue
let length = listeners.length
for (let i = 0; i < length && value === nextValue; i++) {
for (let i = 0; i < length && value === nextValue; i++) { // eslint-disable-line
Copy link
Member Author

Choose a reason for hiding this comment

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

can confirm that removing value === nextValue here breaks tests.
standard is correct that value/nextValue are not mutated in this loop, however value can be mutated by set while these loops are running, so yeah

@mixmix mixmix requested a review from staltz September 28, 2023 22:42
staltz
staltz approved these changes Sep 29, 2023
View reviewed changes
Copy link
Member

@staltz staltz left a comment

Choose a reason for hiding this comment

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

Good idea. I think the code looks correct. I also ran the test suite locally

Base automatically changed from once.remove to master October 1, 2023 23:56
@mixmix mixmix merged commit 34cd2e3 into master Oct 1, 2023
1 check passed
@mixmix mixmix deleted the standard branch October 1, 2023 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@staltz staltz staltz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mixmix @staltz