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

<JSX> ☲ "Simplified" version of Sander's diff. #775

Merged
merged 2 commits into from
Sep 21, 2016
Merged

Conversation

jordwalke
Copy link
Member

@jordwalke jordwalke commented Sep 20, 2016

Summary: This is a version of Sander's JSX Pull Request, but rebased on top of our recent large refactors, and modified to treat
JSX as "simple" constructs, meaning they are "clearly one unit", and do
not require parenthesis to be wrapped around them in order to be passed
as function arguments. That means you can treat <tag ...> as the
"opening paren" and "" as the closing paren.

This diff also supports:

  • Frags (which are simply lists that don't require commas separating
    contents).
  • Lists of JSX items
let myFunction =
   callSomeFunction
     <FirstItem>foo</FirstItem>
     <SecondItem>foo</SecondItem>;

let lst = [<Tag />, <AnotherTag />];

let lst =
  <>
    <Tag />
    <AnotherTag />
  </>;

Whats Not Implemented: Currently, the limitation of this approach is
that subsequent JSX elements must always have spaces separating them.
I have a good idea about how to solve that limitation, but we can start
using this right now, in an experimental mode.

let items = <Wrapper> <This /> <Must /> <Have /> <Spaces /> </Wrapper>

Test Plan:Test cases.

Reviewers:

Summary: This is a version of Sander's JSX implementation which treats
JSX as "simple" constructs, meaning they are "clearly one unit", and do
not require parenthesis to be wrapped around them in order to be passed
as function arguments. That means you can treat <tag ...> as the
"opening paren" and "</tag>" as the closing paren.

This diff also supports:

   - Frags (which are simply lists that don't require commas separating
     contents).

   - Lists of JSX items

```
let myFunction =
   callSomeFunction
     <FirstItem>foo</FirstItem>
     <SecondItem>foo</SecondItem>;

let lst = [<Tag />, <AnotherTag />];

let lst =
  <>
    <Tag />
    <AnotherTag />
  </>;

```

Whats Not Implemented: Currently, the limitation of this approach is
that subsequent JSX elements must always have spaces separating them.
I have a good idea about how to solve that limitation, but we can start
using this right now, in an experimental mode.

```
let items = <Wrapper> <This /> <Must /> <Have /> <Spaces /> </Wrapper>
```

Test Plan:Test cases.

Reviewers:

CC: @vjeux
Copy link
Contributor

@bsansouci bsansouci left a comment

Choose a reason for hiding this comment

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


let lowerCase = <div argument1=1 />;

let a = <Foo a>5 </Foo>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the string 5, or seen as {5}? (maybe this doesn't matter here actually...)

Copy link
Member Author

@jordwalke jordwalke Sep 20, 2016

Choose a reason for hiding this comment

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

It's the number 5. We may want to change how children are represented, or at least printed. Right now, the current JavaScript JSX conventions are a special case of what's supported. In Reason, the expression {5} is the same as 5 (verify for yourself - it's just a sequence expression with one item returned). So we could print it as <Foo>{5}</Foo>


let a = <Foo a>5 </Foo>;

let a = <Foo a>0.55 </Foo>;
Copy link
Contributor

Choose a reason for hiding this comment

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

<Foo a> two spaces?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, that seems to be a mistake. Will fix.

the issue alltogether.

first token
/ \
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be aligned with <ident and args />?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

@jordwalke
Copy link
Member Author

@bsansouci Want to start adapting this to Cheng Lou's needs?

@jordwalke jordwalke changed the title [JSX] JSX: "Simplified" version of Sander's diff. <JSX> ☲ "Simplified" version of Sander's diff. Sep 20, 2016
@jordwalke
Copy link
Member Author

I'll commit this, and then if anyone has issues with it, I'll gladly fix them or revert it. Ben needs this in to rebase his changes off of.

@jordwalke jordwalke merged commit 926f209 into master Sep 21, 2016
@SanderSpies SanderSpies mentioned this pull request Sep 21, 2016
@chenglou chenglou deleted the JordwalkeJSX branch September 23, 2016 00:08
bsansouci pushed a commit to bsansouci/reason that referenced this pull request Sep 29, 2016
* [JSX] JSX: "Simplified" version of Sander's diff.

Summary: This is a version of Sander's JSX implementation which treats
JSX as "simple" constructs, meaning they are "clearly one unit", and do
not require parenthesis to be wrapped around them in order to be passed
as function arguments. That means you can treat <tag ...> as the
"opening paren" and "</tag>" as the closing paren.

This diff also supports:

   - Frags (which are simply lists that don't require commas separating
     contents).

   - Lists of JSX items

```
let myFunction =
   callSomeFunction
     <FirstItem>foo</FirstItem>
     <SecondItem>foo</SecondItem>;

let lst = [<Tag />, <AnotherTag />];

let lst =
  <>
    <Tag />
    <AnotherTag />
  </>;

```

Whats Not Implemented: Currently, the limitation of this approach is
that subsequent JSX elements must always have spaces separating them.
I have a good idea about how to solve that limitation, but we can start
using this right now, in an experimental mode.

```
let items = <Wrapper> <This /> <Must /> <Have /> <Spaces /> </Wrapper>
```

Test Plan:Test cases.

Reviewers:

CC: @vjeux

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

Successfully merging this pull request may close these issues.

2 participants