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

fix for Issue #371 #380

Closed
wants to merge 2 commits into from
Closed

fix for Issue #371 #380

wants to merge 2 commits into from

Conversation

jmls
Copy link

@jmls jmls commented Mar 9, 2017

What type of pull request are you creating?

  • Bug Fix
  • Enhancement
  • Documentation

How many unit test did you write for this pull request?

none

Write a reason if none (e.g just fixed a typo):
program compiles under 2.2 with update, does not without

Please add a description for your pull request:

fixes the issue #371 Operator '!==' cannot be applied to types compile error with typescript 2.2+

Copy link
Collaborator

@jonathan-casarrubias jonathan-casarrubias left a comment

Choose a reason for hiding this comment

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

👍

@jonathan-casarrubias
Copy link
Collaborator

Thanks for adding this patch, would you mind to add it to development instead of master?

Thanks

@lygstate
Copy link
Member

This patch doesn't fixes #317, in Javascript world, there is no type with name 'array'
so typeof Value !== 'array' are invalid

@jonathan-casarrubias
Copy link
Collaborator

Lol this is getting complicated because there are many multiple issues to be fixed.

We need to fix the typeof === 'array' (correct) changing it for Array.isArray but we also need to get rid of the typeof because of the compilation issue.

Maybe if this issue also adds the Array.isArray will include.

Or I can integrate 1 of these pull requests to development and then the other can update the other pull request with the new changes.

Please share your thoughts since you are fixing different issues on the same piece of code

@lygstate
Copy link
Member

@jonathan-casarrubias, About the function _parseParam fixes, mine is the fixes, and for other, I am not sure what's the meaning.

@jonathan-casarrubias
Copy link
Collaborator

yes I'm just talking about _parseParam fixes. From this pull request we need

let typeofValue:string = typeof value;

and its implementation, and from yours we need the

Array.isArray()

but I see you @lygstate also change the way the the params are build, so maybe we just need the Array.isArray from yours?

@lygstate
Copy link
Member

@jonathan-casarrubias Yes, you can cherry pick that for fixes #371

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

Successfully merging this pull request may close these issues.

3 participants