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

Port examples to use Create React App #1883

Merged
merged 15 commits into from
Sep 2, 2016
Merged

Port examples to use Create React App #1883

merged 15 commits into from
Sep 2, 2016

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Aug 7, 2016

Work in progress.
This is a good opportunity to dogfood the tool!

@mxstbr
Copy link
Contributor

mxstbr commented Aug 7, 2016

RIP #1800

@gaearon
Copy link
Contributor Author

gaearon commented Aug 7, 2016

😅

@canibanoglu
Copy link
Contributor

@gaearon I know you're working on this and I have a suggested improvement. Should I go ahead an create a PR myself?

@gaearon
Copy link
Contributor Author

gaearon commented Aug 10, 2016

What kind of improvement?

@canibanoglu
Copy link
Contributor

Well improvement might not really be the word :) There's an unused import in todo-with-undo's index.js file: import 'babel-polyfill'.

Another thing that caught my eye is the use of double quotes in components/Footer.js. Everywhere strings are enclosed in single quotes but in this file they are enclosed in double quotes. Props passed to FilterLink are probably intentional but I'm not so sure about the "separator" strings {", "} and {" "}.

@gaearon
Copy link
Contributor Author

gaearon commented Aug 10, 2016

There's an unused import in todo-with-undo's index.js file: import 'babel-polyfill'.

Feel free to send a PR for this.

Another thing that caught my eye is the use of double quotes in components/Footer.js. Everywhere strings are enclosed in single quotes but in this file they are enclosed in double quotes. Props passed to FilterLink are probably intentional but I'm not so sure about the "separator" strings {", "} and {" "}.

I'll clean up style according to our lint config before merging.

@canibanoglu
Copy link
Contributor

Great, thanks a lot for the help

@timdorr timdorr mentioned this pull request Aug 10, 2016
@timdorr
Copy link
Member

timdorr commented Aug 10, 2016

Hmm, not sure why that last change didn't fix the build. Well, I tried!

@canibanoglu
Copy link
Contributor

I have noticed that every component under the shopping cart example is a stateless component but they all are defined with the class syntax instead of functional components. I would love to convert everything to functional components if that is alright.

@canibanoglu
Copy link
Contributor

I found some more stuff in the shopping cart example. But I don't really feel comfortable hijacking @gaearon's PR with my questions all the time and I will go through all of the examples in detail during the weekend.
Should I just create PRs for each example and then my proposed changes could be reviewed under those? Or maybe under issues?
I would greatly appreciate your input on this :)

@timdorr
Copy link
Member

timdorr commented Aug 12, 2016

@canibanoglu I left the branch in #1800 untouched. So if you see anything in there you want to pull from, that's fine. We'll do a PR to update the examples after this one cleans up the build infrastructure.

@canibanoglu
Copy link
Contributor

Alright I'll send my PRs to that branch then. Thank you very much for you help!

@gaearon
Copy link
Contributor Author

gaearon commented Sep 2, 2016

Going to Just Do This.
If something’s wrong please fix 😄 .

@gaearon gaearon merged commit 7296d3c into master Sep 2, 2016
@timdorr timdorr deleted the create-react-app branch September 3, 2016 00:58
@gaearon gaearon mentioned this pull request Sep 3, 2016
seantcoyote pushed a commit to seantcoyote/redux that referenced this pull request Jan 14, 2018
* Rename examples => examples_old

* Port counter and counter-vanilla

* Port async example

* Port todos example

* Port todos-with-undo example

* Port some tests over

* Add todomvc example

* Ported shopping-cart example

* Ported tree-view

* Ported real-world

* Add missing dep

* Temp fix install until examples get an equivalent to buildAll.js

* Restore example build scripts.

* Bump react-scripts

* Finish porting examples
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.

4 participants