Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bugfix] fix imports for react-native-web #2797

Merged

Conversation

FiberJW
Copy link
Contributor

@FiberJW FiberJW commented Oct 9, 2019

Hello 馃憢馃従,

This bug surfaced as a result of trying to use styled-components in an Expo universal project (React Native + React Native Web). react-native-web doesn't use default exports like react-native official does, so importing using ES6 default imports breaks it for that react native renderer (TypeError: Cannot read property 'View' of undefined). Using require creates the desired behavior both in react-native and react-native-web without tripping up the navigator is deprecated and has been removed from this package. invariant that pops up when trying to use import * as reactNative from 'react-native'.

It's a pretty simple fix and all of the tests passed, so I hope that this is easy to review and possibly merge.

Thanks!

fixes #2624

-- Juwan Wheatley (@FiberJW)

React Native Web doesn't use default exports, so importing using ES6 imports breaks it for that react-native renderer. Using require creates the same behavior in react-native and react-native-web without tripping up the navigator invariant that pops up when trying to import  * as reactNative from 'react-native'.
@probablyup
Copy link
Contributor

@probablyup probablyup commented Oct 10, 2019

LGTM but I'd like to publish it as a test version for people to try out before we merge into master

@probablyup
Copy link
Contributor

@probablyup probablyup commented Oct 10, 2019

Try 4.4.0-reactnativewebfix and let me know!

@FiberJW
Copy link
Contributor Author

@FiberJW FiberJW commented Oct 10, 2019

Worked for me!

Steps:

  1. yarn global add expo-cli
  2. expo init scrnw-test
  3. cd scrnw-test && yarn add styled-components@test
  4. Change container view from RN StyleSheet to styled component using import styled from 'styled-components/native'
  5. expo start -w
  6. Expo CLI launches browser with RN web project. It works as expected
  7. Open up Expo Client on a mobile device and open your app there. It works as expected
  8. Profit 馃挴

@jtparrett
Copy link

@jtparrett jtparrett commented Oct 10, 2019

This worked perfectly for me 馃檶鉂わ笍

@dbritto-dev
Copy link

@dbritto-dev dbritto-dev commented Oct 10, 2019

@FiberJW can you try with this:

import reactNative from 'react-native'
const { StyleSheet } = reactNative;

@daniellmartins
Copy link

@daniellmartins daniellmartins commented Oct 10, 2019

Doesn't work with SSR, for example Next.js

@FiberJW
Copy link
Contributor Author

@FiberJW FiberJW commented Oct 10, 2019

@danilobrinu

@FiberJW can you try with this:

import reactNative from 'react-native'
const { StyleSheet } = reactNative;

https://github.com/necolas/react-native-web/blob/master/packages/react-native-web/src/index.js

@FiberJW
Copy link
Contributor Author

@FiberJW FiberJW commented Oct 10, 2019

@daniellmartins you're using Next.js+React Native Web+Styled Components? Not sure how because it doesn't work on master. This doesn't break any functionality with Next.JS; it creates the possibility to use it with Next.JS.

@probablyup
Copy link
Contributor

@probablyup probablyup commented Oct 22, 2019

@FiberJW can you add a changelog entry and a similar PR for canary? thanks!

@probablyup probablyup added the 4.0 label Oct 22, 2019
@FiberJW
Copy link
Contributor Author

@FiberJW FiberJW commented Oct 23, 2019

done 馃憤

@probablyup probablyup merged commit 4cf2c5a into styled-components:master Oct 28, 2019
1 check passed
@FiberJW
Copy link
Contributor Author

@FiberJW FiberJW commented Oct 28, 2019

Now that I look at it, this fix is already on Canary. I closed my PR accordingly. Thanks!

@ZiiMakc
Copy link

@ZiiMakc ZiiMakc commented Dec 8, 2019

But what about @types/styled-components? I still have bunch of errors on backend:

UPD: it's still not fixed, for now you can use: skipLibCheck: false for tsconfig.

Issue: 33015

../../node_modules/@types/react-native/globals.d.ts:40:15 - error TS2300: Duplicate identifier 'FormData'.  

40 declare class FormData {
                 ~~~~~~~~

  ../../node_modules/typescript/lib/lib.dom.d.ts:5657:11
    5657 interface FormData {
                   ~~~~~~~~
    'FormData' was also declared here.
  ../../node_modules/typescript/lib/lib.dom.d.ts:5667:13
    5667 declare var FormData: {
                     ~~~~~~~~
  // and many more

  ../../node_modules/@types/react-native/globals.d.ts:258:15
    258 declare class URLSearchParams {
                      ~~~~~~~~~~~~~~~
    'URLSearchParams' was also declared here.

@rgavinc
Copy link

@rgavinc rgavinc commented Jan 9, 2020

Doesn't work with SSR, for example Next.js

@daniellmartins I'm experiencing the same problem, @FiberJW 's solution doesn't work with @expo/next-adapter. Were you able to find a solution?

@Aleksion
Copy link

@Aleksion Aleksion commented Jan 16, 2020

@MrLoh @FiberJW

While styled-components/native "works" with react-native web in it's curren't implementation, it adds a SIGNIFICAN'T chunk to the final bundle due to the way react-native is imported. Basically the require + this:

aliases.split(/\s+/m).forEach(function (alias) {
  return Object.defineProperty(styled, alias, {
    enumerable: true,
    configurable: false,
    get: function get() {
      return styled(reactNative[alias]);
    }
  });
});

I would wager that most developers only use a fraction of react-native-web, but if you include ANY reference to styled-components/native it adds 200-300kb to the gzipped bundle.
My solution so far is to ONLY import the stylesheet and comment out the alias function above. And then I manually wrap components as I need them styled(View)

My question though is if we could do this in a smarter way?

  • Would importing the modules one by one manually (The aliases are written down anyways), enable treeshaking to work properly?
  • Can the get function in the above code work asynchronously with a lazy import?
  • Could we make it possible to OPT-OUT of having all the components added the the global styled object, so users don't have to patch the library manually?

I'm happy to do the work on this, I just wanted to check if anyone had the answers to the above before I jump into it.

@FiberJW
Copy link
Contributor Author

@FiberJW FiberJW commented Jan 16, 2020

I think people would love for someone to figure out how to get tree-shaking working correctly. I don't currently have a solution for it and not much bandwidth to allocate towards such a feat, so I fully invite you to take on the challenge if you're up for it! This patch was just to get SC with RNW from broken to working, and it could definitely improve from here 馃憤

@nandorojo
Copy link

@nandorojo nandorojo commented Jun 19, 2020

Just wondering, has anyone figured out a solution to this problem?

@MrLoh
Copy link
Contributor

@MrLoh MrLoh commented Jun 22, 2020

@nandorojo yes styled(View)

@nandorojo
Copy link

@nandorojo nandorojo commented Jun 22, 2020

@MrLoh do you import styled from native, or normal styled components?

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.

10 participants