React-Feature-Toggles #106
Conversation
|
|
||
| function mergeFeatures( | ||
| currentFeatures: ReadonlyArray<string>, | ||
| restOfFeatures: ReadonlyArray<string> |
There was a problem hiding this comment.
restOfFeatures should be:
...restOfFeatures: ReadonlyArray<string>
I am not sure how to do the rest operator.
There was a problem hiding this comment.
Have you tried ...restOfFeatures: string[]? I think ReadonlyArray is unsupported in TS for rest params
There was a problem hiding this comment.
There was a problem hiding this comment.
Ok cool! That seems to have worked! I noticed that the ReadonlyArray wasn't an option, just wasn't able to figure it out/find the answer after that. Thank you!
There was a problem hiding this comment.
Well, it worked on my local machine, but out lint rule changes it to ReadonlyArray<string> and that doesn't work.
There was a problem hiding this comment.
I think it's possible to avoid this rule in this line if you need
There was a problem hiding this comment.
I updated to ...restOfFeatures: string[] with the rule, but it is giving this error: Argument of type 'string[]' is not assignable to parameter of type 'string'. Trying to work that out before I merge.
There was a problem hiding this comment.
I think the correct solution was ...restOfFeatures: Array<ReadonlyArray<string>>
There was a problem hiding this comment.
LGTM @lautarodragan do you agree?
| function getReqQueryFeatures(req?: Req): ReadonlyArray<string> | ||
| export { getReqQueryFeatures } | ||
|
|
||
| function getBrowserQueryFeatures(search?: string): ReadonlyArray<string> |
There was a problem hiding this comment.
This was not working for some reason. I am having trouble with it.
There was a problem hiding this comment.
What problem are you having with this one?
There was a problem hiding this comment.
I'm honestly not really sure what is going on. I thought that I had the types correct and everything, but whenever I'm trying to test to see if it actually works it says that it isn't a function. TypeError: react_feature_toggles_1.getBrowserQueryFeatures is not a function
There was a problem hiding this comment.
This was a problem with react-feature-toggles, I fixed it over there.
|
wooow React 16.4.1, I like so much this PR @jwicks31 |
| @@ -16,7 +16,7 @@ storiesOf('Components/Text', module).addWithJSX( | |||
| ### Usage | |||
| ~~~js | |||
| <Hash | |||
| readonly children?: JSX.Element | |||
| readonly children?: any | |||
There was a problem hiding this comment.
Could you add information about this change?
There was a problem hiding this comment.
It was not building after I had updated React. At first some things I read where saying that the recommendation was to use children?: any, but I read a little more over the weekend and I believe React.ReactNode is the correct type for most of them. There are some that could possibly just be strings? But I wasn't sure how to handle them Ex. Hash and HeaderTitle.
|
Nice!
🎆 |
Resolves #105
Description of Changes
Installing react-feature-toggles required updates to a few dependencies.
react&react-domtov16.4.1@types/reacttov16.3.17react-hot-loadertov3.1.3-This is not the most up to date version, but v1 would not work with React 16. The current version does not support webpack.
react-feature-togglesreact-hot-loaderreact-feature-togglestyping declaration.d.tsfile.TO TEST REACT-FEATURE-TOGGLES
Open Netlify Link
Add '?ft=foo' to the end of the url.
PR Review Checklist