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

Add extra.raw back to JSXText and JSXAttribute #344

Merged
merged 2 commits into from Apr 4, 2017

Conversation

rattrayalex
Copy link
Contributor

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.

@rattrayalex
Copy link
Contributor Author

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!

Copy link
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
Contributor Author

rattrayalex commented Mar 16, 2017

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
Contributor Author

rattrayalex commented Mar 16, 2017

Rebased and changed target

@codecov
Copy link

codecov bot commented Mar 16, 2017

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.

@hzoo hzoo changed the base branch from 7.0 to master March 21, 2017 21:11
@hzoo
Copy link
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
Contributor Author

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants