Add more type definitions#882
Conversation
|
ouch, sorry to stumble upon your feet, but note that in PR #881 I'm doing (yet another 😅) heavy refactoring which among the other things kills a lot of code, include some which was also touched by you. |
Haha no worries! I'll wait for that PR to be in and rebase on it 😄 |
|
I'm waiting for #884 to get in before I rebase 😄 |
|
Nice PR! Thanks @FabioRosado
.... yet 😆 |
|
@FabioRosado this should be rebased now since #884 is merged now 😅 |
madhur-tandon
left a comment
There was a problem hiding this comment.
@FabioRosado Yours to merge when the tests pass :)
|
Thanks @madhur-tandon I can't merge it though 😄 Just ran |
Whenever you are ready 💯 |
|
|
| @@ -1,4 +1,4 @@ | |||
| import { addClasses, htmlDecode, ensureUniqueId } from '../utils'; | |||
There was a problem hiding this comment.
@antocuni just checking, should pytitle call ensureUniqueId as well?
There was a problem hiding this comment.
I think that py-title should die in flames 😅.
Looking at the code I think it should because it's indeed using this.id.
But on the other hand, I don't think that the id is used for anything useful, so we could just avoid using it.
There was a problem hiding this comment.
Haha removing things is always fun!
Awesome I just wanted to be sure that I didn't removed something that we would use in the future 😀
|
There are still about 71 warnings but a lot of them are coming from pyodide since most of the methods are typed upstream as |
|
Awesome work |
I started adding some types in an effort to fix #834 but Jeff discovered the issue and fixed it 😄
This PR adds a bit of typing across the board - I'm still learning how to use the more advanced features of typescript, so there are some changes that I am unsure of 🤔
Also I'm not entirely sure if we should replace some/all of these
anyforunknown