[RFR] Migrate ra-core to TypeScript #2508
Conversation
djhi
commented
Nov 7, 2018
•
djhi
commented
- Fix demo apps
- i18n related code
- auth related code
- [x] Fix demo apps - [ ] i18n related code
ghost
changed the title
| componentWillMount() { | ||
| this.checkAuthentication(this.props); | ||
| } | ||
|
|
||
| componentWillReceiveProps(nextProps) { | ||
| if (nextProps.location !== this.props.location) { | ||
| if (!isEqual(nextProps.location, this.props.location)) { |
There was a problem hiding this comment.
React-router's location object is never mutated, better rely on strict equality.
A location object is never mutated so you can use it in the lifecycle hooks to determine when navigation happens
(https://github.com/ReactTraining/react-router/blob/master/packages/react-router/docs/api/location.md)
| @@ -69,5 +70,5 @@ export class Authenticated extends Component { | |||
|
|
|||
| export default connect( | |||
| null, | |||
| { userCheck } | |||
| { userCheck: userCheckAction } | |||
There was a problem hiding this comment.
Why did you rename the action creator in the import? It's cumbersome.
There was a problem hiding this comment.
Because TypeScript otherwise complain about shadowed variable names when we get userCheck from the props through destructuring
| @@ -148,6 +163,6 @@ export default compose( | |||
| }), | |||
| connect( | |||
| mapStateToProps, | |||
| { userCheck } | |||
| { userCheck: userCheckAction } | |||
packages/ra-core/src/types.ts
Outdated
| @@ -0,0 +1,20 @@ | |||
| export type I18nProvider = (locale: string) => object; | |||
There was a problem hiding this comment.
The i18nProvider may return a promise for locale change calls
| @@ -20,7 +20,7 @@ describe('<NullableBooleanInput />', () => { | |||
|
|
|||
| const MenuItemElement1 = MenuItemElements.at(0); | |||
| expect(MenuItemElement1.prop('value')).toEqual(''); | |||
| expect(MenuItemElement1.childAt(0).text()).toEqual(''); | |||
| expect(MenuItemElement1.children().length).toEqual(0); | |||
There was a problem hiding this comment.
that's not the same test. Why did you change it?
There was a problem hiding this comment.
Because it does not have any children when its content is empty. Maybe related to the upgrade of enzyme adapter
| @@ -68,7 +68,7 @@ describe('<SelectInput />', () => { | |||
| assert.equal(MenuItemElements.length, 3); | |||
| const MenuItemElement1 = MenuItemElements.first(); | |||
| assert.equal(MenuItemElement1.prop('value'), ''); | |||
| assert.equal(MenuItemElement1.childAt(0).text(), ''); | |||
| assert.equal(MenuItemElement1.children().length, 0); | |||
|
I updated my package to 2.4.2, then unfortunately the LoginForm's input placeholder can't change.It is 'ra.auth.username' now. When package version is 2.4.1 , the LoginForm's input placeholder is 'Username'. |
|
Please don't ask unrelated questions on pull requests. This makes your question easy to find by the core team, and the developer community. Unlike Github, StackOverFlow has great SEO, gamification, voting, and reputation. That's why we chose it, and decided to keep GitHub issues only for bugs and feature requests. So I invite you to ask your question at: http://stackoverflow.com/questions/tagged/react-admin And once you get a response, please continue to hang out on the react-admin channel in StackOverflow. That way, you can help newcomers and share your expertise! |
|
Sorry. I asked my question at StackOverflow. |
