-
-
Notifications
You must be signed in to change notification settings - Fork 861
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
Fix flow types and enable flow types testing on CI #896
Conversation
0933a51
to
e73f7a5
Compare
@kazupon I see that you have fixed the types (or partly fixed partly ignored). Here is some more fixes so that the Browser tests still crashing, not sure why. |
I'll have to check if this change still allows overriding the protototype. I'm afraid it might not... |
Codecov Report
@@ Coverage Diff @@
## v8.x #896 +/- ##
==========================================
+ Coverage 96.29% 96.31% +0.02%
==========================================
Files 10 10
Lines 891 896 +5
==========================================
+ Hits 858 863 +5
Misses 33 33
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your PR!
It seem to be good. 👍
I've just tried CI, and it's succeeded. (why CI failed, I'm not sure 😅 )
Let me just double-check later that overriding |
Yes, actually that change breaks overriding through prototype. |
The tests were using "assert" function wrong. "assert" is defined as: assert(value, [message]) The tests passed two arguments in an attempt to compare them while the only thing that happened was that the first value was checked for being truthy.
Types fixed by making "getChoiceIndex" method follow types and be a class property rather than class method. Class methods are "read-only" in flow and that wasn't compatible with flow types that are defining that function as a property (to allow overriding through prototype changing).
Now it's ready. |
Thank you very much! |
The change related to
getChoiceIndex
is IMO a correct fix. It's necessary as just declaringgetChoiceIndex
as a method of the class makes itread-only
and that is not compatible with flow types that havegetChoiceIndex
as a property (correctly as this method is overridable throughVueI18n.prototype
).Also enables unit tests and flow types testing on CI