Add a mode to create react-native svg using expo #289

edpaget · 2019-04-03T21:15:42Z

Summary

Using an ejected ExpoKit after version 32 prevents import Svg from 'react-native-svg'; from working, and also prevents the developer from adding
'react-native-svg' as a direct dependency since ExpoKit already provides the
library just behind the ExpoKit svg namespace. See
kristerkari/react-native-svg-transformer#13

This handles that case by allowing a expo: true flag to be passed instead of
the native option and will create the correct import statement to use Expo's
SVG library as well as prefix all non <Svg /> components with Svg..

Test plan

Added an additional test case to show the import statement and the component name prefixing.

Using an ejected ExpoKit after version 32 prevents `import Svg from 'react-native-svg';` from working, and also prevents the developer from adding 'react-native-svg' as a direct dependency since ExpoKit already provides the library just behind the ExpoKit svg namespace. See kristerkari/react-native-svg-transformer#13 This handles that case by allowing a `expo: true` flag to be passed instead of the `native` option and will create the correct import statement to use Expo's SVG library as well as prefix all non `<Svg />` components with `Svg.`.

vercel · 2019-04-03T21:15:47Z

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

codecov · 2019-04-03T21:19:28Z

Codecov Report

Merging #289 into master will increase coverage by 0.07%.
The diff coverage is 86.36%.

@@            Coverage Diff             @@
##           master     #289      +/-   ##
==========================================
+ Coverage    85.3%   85.37%   +0.07%     
==========================================
  Files          30       30              
  Lines         449      458       +9     
  Branches      125      129       +4     
==========================================
+ Hits          383      391       +8     
- Misses         55       56       +1     
  Partials       11       11

Impacted Files	Coverage Δ
...s/babel-plugin-transform-svg-component/src/util.js	`75% <50%> (-1.67%)`	⬇️
packages/babel-preset/src/index.js	`86.2% <50%> (ø)`	⬆️
...bel-plugin-transform-react-native-svg/src/index.js	`97.72% <94.44%> (+0.42%)`	⬆️

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 f476c8e...9ac366f. Read the comment docs.

codecov · 2019-04-03T21:19:28Z

Codecov Report

Merging #289 into master will increase coverage by 0.07%.
The diff coverage is 86.36%.

@@            Coverage Diff             @@
##           master     #289      +/-   ##
==========================================
+ Coverage    85.3%   85.37%   +0.07%     
==========================================
  Files          30       30              
  Lines         449      458       +9     
  Branches      125      129       +4     
==========================================
+ Hits          383      391       +8     
- Misses         55       56       +1     
  Partials       11       11

Impacted Files	Coverage Δ
...s/babel-plugin-transform-svg-component/src/util.js	`75% <50%> (-1.67%)`	⬇️
packages/babel-preset/src/index.js	`86.2% <50%> (ø)`	⬆️
...bel-plugin-transform-react-native-svg/src/index.js	`97.72% <94.44%> (+0.42%)`	⬆️

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 f476c8e...9ac366f. Read the comment docs.

gregberge

Hello @edpaget, thanks for this PR! Just a small change in the preset API. Also please check if readmes have to be updated.

gregberge · 2019-04-05T18:51:07Z

packages/babel-preset/src/index.js

-  if (opts.native) {
-    plugins.push(transformReactNativeSVG)
+  if (opts.native || opts.expo) {
+    plugins.push([transformReactNativeSVG, { expo: !!opts.expo }])


Hmm, I think we should use native: { expo: true } instead of a direct expo option.

codecov-io · 2019-04-06T17:14:15Z

Codecov Report

Merging #289 into master will increase coverage by 0.07%.
The diff coverage is 86.36%.

@@            Coverage Diff             @@
##           master     #289      +/-   ##
==========================================
+ Coverage    85.3%   85.37%   +0.07%     
==========================================
  Files          30       30              
  Lines         449      458       +9     
  Branches      125      129       +4     
==========================================
+ Hits          383      391       +8     
- Misses         55       56       +1     
  Partials       11       11

Impacted Files	Coverage Δ
...s/babel-plugin-transform-svg-component/src/util.js	`75% <50%> (-1.67%)`	⬇️
packages/babel-preset/src/index.js	`86.2% <50%> (ø)`	⬆️
...bel-plugin-transform-react-native-svg/src/index.js	`97.72% <94.44%> (+0.42%)`	⬆️

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 f476c8e...e86ae78. Read the comment docs.

codecov-io · 2019-04-06T17:14:15Z

Codecov Report

Merging #289 into master will increase coverage by 0.56%.
The diff coverage is 91.3%.

@@            Coverage Diff             @@
##           master     #289      +/-   ##
==========================================
+ Coverage    85.3%   85.86%   +0.56%     
==========================================
  Files          30       30              
  Lines         449      460      +11     
  Branches      125      130       +5     
==========================================
+ Hits          383      395      +12     
+ Misses         55       54       -1     
  Partials       11       11

Impacted Files	Coverage Δ
...s/babel-plugin-transform-svg-component/src/util.js	`81.25% <100%> (+4.58%)`	⬆️
packages/babel-preset/src/index.js	`87.09% <66.66%> (+0.88%)`	⬆️
...bel-plugin-transform-react-native-svg/src/index.js	`97.72% <94.11%> (+0.42%)`	⬆️

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 f476c8e...c5c7523. Read the comment docs.

edpaget · 2019-04-06T17:15:49Z

Thanks for the review @neoziro (and for the library!). I've change the option to expect the native: { expo: true } argument and updated the options documentation. I'm not sure if there's another place that should be added in the documentation.

gregberge · 2019-04-09T07:01:29Z

Thanks @edpaget, could you fix tests please? Also please run yarn format, thanks!

Run yarn format

gregberge · 2019-04-10T14:51:38Z

Thanks!

vercel bot deployed to staging April 3, 2019 21:15 View deployment

edpaget mentioned this pull request Apr 3, 2019

react-native-svg-transformer not working with expo kristerkari/react-native-svg-transformer#13

Closed

gregberge requested changes Apr 5, 2019

View reviewed changes

Update options documention to describe native: { expo: true }

4b48f4c

vercel bot temporarily deployed to staging April 6, 2019 17:14 Inactive

edpaget force-pushed the ep-add-support-for-expokit-svg branch from e86ae78 to 31849a6 Compare April 6, 2019 17:15

vercel bot temporarily deployed to staging April 6, 2019 17:15 Inactive

Pass in opts.native argument as options to react-native transformer

dd8fd78

edpaget force-pushed the ep-add-support-for-expokit-svg branch from 31849a6 to dd8fd78 Compare April 6, 2019 17:16

vercel bot deployed to staging April 6, 2019 17:16 View deployment

Correct how options get passed to native plugin

a44d38f

Run yarn format

vercel bot deployed to staging April 9, 2019 13:33 View deployment

Add additional tests

cb0f445

vercel bot deployed to staging April 9, 2019 14:00 View deployment

Add additional tests to the transform svg component

549bc27

vercel bot deployed to staging April 9, 2019 15:06 View deployment

Rerun yarn format

c5c7523

vercel bot deployed to staging April 9, 2019 15:13 View deployment

gregberge merged commit 978db3e into gregberge:master Apr 10, 2019

edpaget deleted the ep-add-support-for-expokit-svg branch April 10, 2019 15:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a mode to create react-native svg using expo #289

Add a mode to create react-native svg using expo #289

edpaget commented Apr 3, 2019 •

edited

vercel bot commented Apr 3, 2019 •

edited

codecov bot commented Apr 3, 2019

codecov bot commented Apr 3, 2019 •

edited

gregberge left a comment

gregberge Apr 5, 2019

codecov-io commented Apr 6, 2019

codecov-io commented Apr 6, 2019 •

edited by codecov bot

edpaget commented Apr 6, 2019 •

edited

gregberge commented Apr 9, 2019

gregberge commented Apr 10, 2019

Add a mode to create react-native svg using expo #289

Add a mode to create react-native svg using expo #289

Conversation

edpaget commented Apr 3, 2019 • edited

Summary

Test plan

vercel bot commented Apr 3, 2019 • edited

codecov bot commented Apr 3, 2019

Codecov Report

codecov bot commented Apr 3, 2019 • edited

Codecov Report

gregberge left a comment

Choose a reason for hiding this comment

gregberge Apr 5, 2019

Choose a reason for hiding this comment

codecov-io commented Apr 6, 2019

Codecov Report

codecov-io commented Apr 6, 2019 • edited by codecov bot

Codecov Report

edpaget commented Apr 6, 2019 • edited

gregberge commented Apr 9, 2019

gregberge commented Apr 10, 2019

edpaget commented Apr 3, 2019 •

edited

vercel bot commented Apr 3, 2019 •

edited

codecov bot commented Apr 3, 2019 •

edited

codecov-io commented Apr 6, 2019 •

edited by codecov bot

edpaget commented Apr 6, 2019 •

edited