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

Add eslint to project and fix problems #287

Closed
wants to merge 1 commit into from

Conversation

antonilol
Copy link
Contributor

@antonilol antonilol commented Feb 12, 2022

closes #286

@antonilol antonilol marked this pull request as ready for review February 13, 2022 18:44
src/utils/cert.ts Outdated Show resolved Hide resolved
src/utils/msg.ts Outdated Show resolved Hide resolved
@antonilol

This comment was marked as resolved.

@antonilol

This comment was marked as resolved.

@antonilol

This comment was marked as resolved.

@antonilol

This comment was marked as resolved.

@antonilol
Copy link
Contributor Author

ts complains that mediaKeyMap can be undefined because it is only set if (m.type === constants.message_types.attachment)

chatTribes.ts lines 682-683

isTribeOwner?: boolean
dest?: string
owner?: any
bot_id?: any
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: bot_id changed from string to any

@antonilol
Copy link
Contributor Author

@antonilol
Copy link
Contributor Author

src/notify.ts:105:17 - error TS2339: Property 'blocked' does not exist on type 'Contact'.

105       if (other.blocked) return

add it?

@antonilol
Copy link
Contributor Author

src/scopes.ts:22:22 - error TS2339: Property 'scope' does not exist on type 'JwtBody'.

22   const scopes = jwt.scope.split(',')

this function is given a JwtBody elsewhere in the repo so i assigned that type to the parameter but ts is not happy

@antonilol
Copy link
Contributor Author

there are a lot of errors when running npm run build. lots of them are caused by ts not knowing Model.update (where model is Message or Chat etc)
how to fix this?

@antonilol antonilol closed this Feb 20, 2022
@antonilol antonilol reopened this Feb 20, 2022
@antonilol
Copy link
Contributor Author

oops wrong button

@antonilol

This comment was marked as resolved.

@antonilol
Copy link
Contributor Author

In file helpers.ts
On line 265 owner is checked if it is undefined, and then reassigned with a Contact.dataValues. note that after the if block owner can be a Contact or Contact.dataValues. on line 279 owner.dataValues is passed into a function. This can be a Contact.dataValues.dataValues, that's undefined, right?

@Evanfeenstra
Copy link
Contributor

In file helpers.ts On line 265 owner is checked if it is undefined, and then reassigned with a Contact.dataValues. note that after the if block owner can be a Contact or Contact.dataValues. on line 279 owner.dataValues is passed into a function. This can be a Contact.dataValues.dataValues, that's undefined, right?

Right, 269 shouldn't have the .dataValues. However this code is many month old and has never caused an issue (I think dat.owner is always defined), so possibly the problem is somewhere else

@antonilol
Copy link
Contributor Author

Maybe dat.owner only is undefined in some test case, because the logs kevin sent show an undefined owner being passed in a function (property id of undefined)

@antonilol
Copy link
Contributor Author

or Payload.owner is a Contact.dataValues
why is the owner.dataValues passed into the function and not owner?

@antonilol
Copy link
Contributor Author

antonilol commented Mar 4, 2022

https://github.com/stakwork/sphinx-relay/blob/master/src/tests/controllers/clearAllChats.test.ts#L30-L31
chats is a Chat[], c (from the asyncForEach) is the entry value, so Chat and is passed in a function that needs a number
looking at the logs of the test it doesn't get here (so if there are errors here, they simply dont appear)

alice had no chats
bob had no chats
carol had no chats

i changed it to c.id (the argument is chatID)

@antonilol
Copy link
Contributor Author

will be merging and resolving conflicts here to have a diff of all this work between master every now and then when kevin merges a part with e.g. #307
but cant push atm super annoying

Enumerating objects: 258, done.
Counting objects: 100% (258/258), done.
Delta compression using up to 12 threads
Compressing objects: 100% (87/87), done.
Writing objects: 100% (88/88), 17.92 KiB | 2.99 MiB/s, done.
Total 88 (delta 74), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (74/74), completed with 74 local objects.
remote: Internal Server Error
To github.com:antonilol/sphinx-relay.git
 ! [remote rejected]   eslint -> eslint (Internal Server Error)
error: failed to push some refs to 'github.com:antonilol/sphinx-relay.git'

antonilol added a commit to antonilol/sphinx-relay that referenced this pull request Mar 31, 2022
antonilol added a commit to antonilol/sphinx-relay that referenced this pull request May 12, 2022
antonilol added a commit to antonilol/sphinx-relay that referenced this pull request May 12, 2022
antonilol added a commit to antonilol/sphinx-relay that referenced this pull request May 12, 2022
lower warning count to 466
@antonilol antonilol mentioned this pull request May 25, 2022
antonilol added a commit to antonilol/sphinx-relay that referenced this pull request Jun 6, 2022
lower warning count to 466
@antonilol antonilol marked this pull request as draft June 8, 2022 16:15
antonilol added a commit to antonilol/sphinx-relay that referenced this pull request Jun 13, 2022
lower warning count to 466
@kevkevinpal
Copy link
Contributor

closing this PR since we've got a system working for eslint now where we decrement the warning count and slowly make our way down

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

Successfully merging this pull request may close these issues.

Eslint checks on the sphinx relay repo - $@ 700,000
3 participants