Skip to content

Make it work with react native 0.51 #13

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

infostreams
Copy link

Did not work (at all) before

@infostreams infostreams changed the title Made it work with react native 0.51 Make it work with react native 0.51 Jan 9, 2018
"main": "dist/react-intl-native.js",
"jsnext:main": "dist/react-intl-native.es2015.js",
"main": "src/index.js",
"jsnext:main": "src/index.js",
Copy link
Owner

Choose a reason for hiding this comment

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

jsnext:main is deprecated. I don't think we should use it. If we need an ES2015 version to be explicitly defined, it should be under module.

"main": "dist/react-intl-native.js",
"jsnext:main": "dist/react-intl-native.es2015.js",
"main": "src/index.js",
"jsnext:main": "src/index.js",
"scripts": {
"prebuild": "rimraf dist",
"build": "rollup-babel-lib-bundler -f cjs,es6 src/index.js",
Copy link
Owner

Choose a reason for hiding this comment

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

Since it's directly referencing the source, we don't need the rollup builds any more. I'm not sure if it's in the scope of the pull request though.

@@ -2,7 +2,7 @@

[![Build Status](https://travis-ci.org/frostney/react-intl-native.svg?branch=master)](https://travis-ci.org/frostney/react-intl-native) [![Dependency Status](https://david-dm.org/frostney/react-intl-native.svg)](https://david-dm.org/frostney/react-intl-native) [![devDependency Status](https://david-dm.org/frostney/react-intl-native/dev-status.svg)](https://david-dm.org/frostney/react-intl-native#info=devDependencies) [![codecov.io](https://codecov.io/github/frostney/react-intl-native/coverage.svg?branch=master)](https://codecov.io/github/frostney/react-intl-native?branch=master)

`react-intl` convinience components for React Native
`react-intl` convenience components for React Native
Copy link
Owner

Choose a reason for hiding this comment

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

👍

@@ -10,6 +11,21 @@ const FormattedDate = props => (

FormattedDate.propTypes = {
style: PropTypes.any,
localeMatcher: PropTypes.oneOf(['best fit', 'lookup']),
Copy link
Owner

Choose a reason for hiding this comment

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

👍 for writing all of these up

@infostreams
Copy link
Author

Thanks! So shall I remove jsnext:main and revert back to using dist/... instead of src/... then?

@frostney
Copy link
Owner

In my opinion removing the build process and using the ES2015 modules should be part of a separate pull request only scoped on updating the build process. I would like to have jsnext:main have be removed as a property.

Please see as my comments as nitpicks only. It would not block the merging in, once the test passes 🙂
I really appreciate all of the work you are doing with this pull request ❤️

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

Successfully merging this pull request may close these issues.

3 participants