Skip to content
This repository has been archived by the owner on Feb 23, 2021. It is now read-only.

Initial version #1

Merged
merged 1 commit into from
Dec 22, 2014
Merged

Initial version #1

merged 1 commit into from
Dec 22, 2014

Conversation

mlmorg
Copy link
Contributor

@mlmorg mlmorg commented Dec 22, 2014

cc @cain-uber

@Raynos
Copy link

Raynos commented Dec 22, 2014

lgtm.

@cain-wang
Copy link
Contributor

Hi Matt,

Awesome! Thanks for setting up the ground work.
I had some prototype code running, shall I post a code review and we could
discuss it?

Thanks,

Cain

On Sun, Dec 21, 2014 at 6:08 PM, Matt Morgan notifications@github.com
wrote:

cc @cain-uber https://github.com/cain-uber

You can merge this Pull Request by running

git pull https://github.com/uber/r-dom dev/initial

Or view, comment on, or merge it at:

#1
Commit Summary

  • Get basic tag working
  • Add a bunch of tests and get them working
  • Add a bunch of tests for components
  • Add more tests
  • Change name of file
  • Add more key property tests
  • Change test name
  • More test changes
  • Get ready for shipping
  • Add readme

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#1.

@mlmorg
Copy link
Contributor Author

mlmorg commented Dec 22, 2014

Do you want me to merge this first?

@cain-wang
Copy link
Contributor

Oh, I just found out somehow github rejected my review comments earlier ..
I'll just post it here.

I noticed we are having the children passed in as an array into r(),
React.createElement() just assumes whatever follows the attribute config
are child elements, I think that provides a cleaner syntax.

Thanks,

Cain

function createComponent(properties) {
properties = extend({
render: function render() {
return (
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, when we use the JS API to create React element, we don't need to wrap the returned block with (), right?

Yay, github finally lets me post comments :)

@mlmorg mlmorg merged commit 8c5bd34 into master Dec 22, 2014
@dawsbot dawsbot deleted the dev/initial branch April 13, 2017 22:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants