-
Notifications
You must be signed in to change notification settings - Fork 164
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
[web] Added support for Expo web #58
Conversation
@bleonard could you please review this |
Please, we need it for this: |
Any news? |
@@ -5,12 +5,7 @@ | |||
'use strict'; | |||
|
|||
import React from 'react'; | |||
import ReactNative from 'react-native'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any significant changes in this file that need to be reviewed? Most of them look like Formatting changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default export of react-native
has been removed from web and will be removed from native soon. I opened this PR right after deleting it. It was removed because it causes all of the RN module to be bundled regardless of if you are using them or not.
src/ParsedText.js
Outdated
@@ -1,5 +1,5 @@ | |||
import React from 'react'; | |||
import ReactNative from 'react-native'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any significant changes in this file that need to be reviewed? Most of them look like Formatting changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default export of react-native
has been removed from web and will be removed from native soon. I opened this PR right after deleting it. It was removed because it causes all of the RN module to be bundled regardless of if you are using them or not.
bump |
@acidhax my questions are still unanswered. It feels like there are a few too many changes in this PR for what it claims to be doing, and I would prefer more documentation or explanations so that reviewers can verify this works correctly. I’m currently not inclined to merge this PR as is, but if @EvanBacon wants to update it, or if somebody else can make a more targeted/verifiable PR then we might be able to assess this further. |
Thanks for your changes @EvanBacon -- I'm going to try to get this merged this week. |
any news? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @radekcy! Are there any other changes that might be needed?
Co-Authored-By: Frederic Barthelemy <[email protected]>
@fbartho no I haven't encountered any other issues - could it be merged soon? |
Can this please be merged? |
@RoninSTi -- Yeah, we're working on getting a release ready with this PR & with support for react-native 0.60 (probably 0.62). |
@fbartho Is there anything more need to be done to push this PR forward? Thanks. |
Thanks for the reminder! I’ll take a look tomorrow |
Finally working on it now. |
I have this merged into an integration branch, will update when a beta build is out. |
The default ReactNative export was removed from the most recent version of react-native-web. Using
babel-preset-expo
will provide the correct babel config on web and native. This is needed for gettingreact-native-gifted-chat
working in web.