Skip to content
This repository was archived by the owner on May 19, 2018. It is now read-only.

Add extra.raw back to JSXText and JSXAttribute#344

Merged
danez merged 2 commits intobabel:masterfrom
rattrayalex:jsx-escaping
Apr 4, 2017
Merged

Add extra.raw back to JSXText and JSXAttribute#344
danez merged 2 commits intobabel:masterfrom
rattrayalex:jsx-escaping

Conversation

@rattrayalex
Copy link
Copy Markdown
Contributor

@rattrayalex rattrayalex commented Feb 2, 2017

Q A
Bug fix? yes
Breaking change? I don't believe so
New feature? no
Deprecations? no
Spec compliancy? yes
Tests added/pass? yes
Fixed tickets #316 , #271
License MIT

See discussion at #316 (comment) and above.

node.extra was removed in order to detect whether a literal was JSX or not, as a temporary fix.

This hack no longer seems to be necessary.

This was referenced Feb 2, 2017
Merged
Closed
@rattrayalex
Copy link
Copy Markdown
Contributor Author

rattrayalex commented Feb 2, 2017

I'm somewhat concerned that I was inadequately creative in coming up with encoding-related edge-cases, especially given that my knowledge of xhtml-vs-js encoding is limited. Would appreciate any input on that area!

@rattrayalex rattrayalex mentioned this pull request Feb 2, 2017
Closed
danez
danez suggested changes Feb 2, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@danez danez left a comment

Choose a reason for hiding this comment

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

See comment in babel/babel#5256

@rattrayalex
Copy link
Copy Markdown
Contributor Author

rattrayalex commented Mar 16, 2017 •
edited
Loading

As noted in babel/babel#5256 , this should not go out before 7.0.

Can/should I change the merge target @danez ? Can you?

EDIT – I will rebase and change target

@rattrayalex rattrayalex changed the base branch from master to 7.0 March 16, 2017 17:43
@rattrayalex
Copy link
Copy Markdown
Contributor Author

rattrayalex commented Mar 16, 2017 •
edited
Loading

Rebased and changed target

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 16, 2017 •
edited
Loading

Codecov Report

Merging #344 into master will decrease coverage by 16.66%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           master     #344       +/-   ##
===========================================
- Coverage   98.25%   81.59%   -16.67%     
===========================================
  Files          20       20               
  Lines        3503     3499        -4     
  Branches      927      927               
===========================================
- Hits         3442     2855      -587     
- Misses         22      443      +421     
- Partials       39      201      +162
Flag Coverage Δ
#babel 81.59% <100%> (-0.03%) ⬇️
#babylon ?
Impacted Files Coverage Δ
src/plugins/jsx/index.js 79.32% <100%> (-18.46%) ⬇️
src/plugins/estree.js 2.96% <0%> (-96.3%) ⬇️
src/index.js 50% <0%> (-50%) ⬇️
src/parser/lval.js 80.27% <0%> (-18.37%) ⬇️
src/tokenizer/index.js 82.64% <0%> (-15.71%) ⬇️
src/parser/statement.js 83.43% <0%> (-15.65%) ⬇️
src/parser/index.js 86.48% <0%> (-13.52%) ⬇️
src/parser/expression.js 84.92% <0%> (-12.47%) ⬇️
src/util/identifier.js 88.57% <0%> (-11.43%) ⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f98279...650adde. Read the comment docs.

@danez danez mentioned this pull request Mar 21, 2017
Closed
@hzoo hzoo changed the base branch from 7.0 to master March 21, 2017 21:11
@hzoo
Copy link
Copy Markdown
Member

hzoo commented Mar 21, 2017

We moved the previous 7.0 branch to be master now, and master is now 6.x

@rattrayalex
Copy link
Copy Markdown
Contributor Author

rattrayalex commented Mar 21, 2017

Okay. Fixed conflicts – is this ready to be merged then?

@rattrayalex rattrayalex mentioned this pull request Mar 22, 2017
Closed
@ChristianHersevoort ChristianHersevoort mentioned this pull request Mar 24, 2017
Merged
@danez danez merged commit cbf4203 into babel:master Apr 4, 2017
@rattrayalex rattrayalex mentioned this pull request Apr 6, 2017
Merged
@sebmarkbage sebmarkbage mentioned this pull request May 12, 2017
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@existentialism existentialism existentialism approved these changes

@danez danez danez approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

i: enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rattrayalex @hzoo @existentialism @danez @xtuc