godaddy / svgs

svgs is a compatiblity layer between svg and react-native-svg
MIT License
191 stars 31 forks source link

Declare babel dependencies, update es2015 to env #26

Closed Tvrqvoise closed 3 years ago

Tvrqvoise commented 6 years ago

Because this repo declares its babel config in package.json, Babel will attempt to transform this package during runtime using the presets defined there. Because this package does not declare its Babel presets as dependencies, this causes node require errors unless the parent package directly depends on these packages as well:

screen shot 2018-07-09 at 12 21 55 pm

Also, I updated babel-preset-es2015 to babel-preset-env, as this is the new recommendation from the Babel team.

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 17


Totals Coverage Status
Change from base Build 16: 0.0%
Covered Lines: 60
Relevant Lines: 60

💛 - Coveralls
3rd-Eden commented 6 years ago

Thanks for taking the time to create PR, what is the reason from bringing in the devDependencies as dependencies? Wouldn't it make more sense that these are declared on an application level instead of package level? Especially because the package is already providing pre-build versions of the library.

Tvrqvoise commented 6 years ago

@3rd-Eden I forgot to update my description for this PR, but it's there now. Essentially, anything in your babel configuration gets required, and since my package doesn't use these presets itself, it results in require errors.

It's true that this package does run babel before publish, so another alternative here would be to remove the "babel" field from package.json and depend on the package-time transforms only. If you would rather go down this route, I can make those updates.

Tvrqvoise commented 6 years ago

Okay -- I created a new PR which solves this the other way -- don't package the babel configuration, and then we won't need to depend upon it. If you prefer that one, we can close this one in favor of it.