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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support React Native #59

Open
CarlosNZ opened this issue May 10, 2024 · 15 comments
Open

Support React Native #59

CarlosNZ opened this issue May 10, 2024 · 15 comments
Assignees
Labels
enhancement New feature or request user request Requested by user

Comments

@CarlosNZ
Copy link
Owner

Originally posted by @michelcrypt4d4mus in #46 (comment)

ok so now i tried to use your package in my app and got an error: Property 'getComputedStyle' doesn't exist. any ideas? seems like it might be a dependency issue (maybe related to why it wouldn't install easily) but i'm not a javascript developer really...

This is how i'm using it in a file called current_daemon_status.js. I'm certain that what's in serviceStatus is a valid data structure (i can JSON.stringify() it just fine):

export default function CurrentDaemonStatus() {
    <...snipped API retrieval code...>
    return <JsonEditor data={serviceStatus} />;
}

Here is the App.js file:

export default function App() {
    return (
        <Provider store={store}>
            <View style={styles.container}>
                <ImageBackground source={BACKGROUND_IMAGE} imageStyle={styles.bg_image} resizeMode="cover">
                    <CurrentDaemonStatus style={styles.overlay_text}/>
                </ImageBackground>

                <StatusBar style="auto" />
            </View>
        </Provider>
    );
};

my iOS simulator shows this:

Screenshot 2024-05-10 at 4 25 26 AM

STDERR shows:

 ERROR  ReferenceError: Property 'getComputedStyle' doesn't exist

This error is located at:
    in CollectionNode (created by Editor)
    in div (created by Editor)
    in Editor
    in CollapseProvider
    in ThemeProvider
    in Unknown (at current_daemon_status.js:32)
    in CurrentDaemonStatus (at App.js:20)
    in RCTView (at View.js:116)
    in View (at ImageBackground.js:82)
    in ImageBackground (at App.js:17)
    in RCTView (at View.js:116)
    in View (at App.js:15)
    in Provider (at App.js:14)
    in App (at withDevTools.ios.js:29)
    in withDevTools(App) (at renderApplication.js:57)
    in RCTView (at View.js:116)
    in View (at AppContainer.js:127)
    in RCTView (at View.js:116)
    in View (at AppContainer.js:155)
    in AppContainer (at renderApplication.js:50)
    in main(RootComponent) (at renderApplication.js:67), js engine: hermes
@CarlosNZ CarlosNZ added the bug Something isn't working label May 10, 2024
@michelcrypt4d4mus
Copy link

fwiw i removed the style={styles.overlay_text} from the CurrentDaemonStatus component and still got same error... also tried removing the ImageBackground component didn't fix anything.

@CarlosNZ
Copy link
Owner Author

You're using React Native, is that right? I've never tried it with React Native, only in the browser, so there's probably going to be a few differences/considerations that I've missed. I'll try and get a repo set up with it and see how it goes. Thanks for the bug report.

@michelcrypt4d4mus
Copy link

yes, React-Native via expo (which has been a surprisingly smooth so far - expected far more headaches).

fwiw i removed the @testing-library/react package from my app's dev dependencies (the package that caused me to have to use --legacy-peer-deps to install json-edit-react successfully). once that was gone i was able to install json-edit-react without having to restort to the dark art of --legacy-peer-deps but i still got the same error.

@michelcrypt4d4mus
Copy link

i see that your code calls getComputedStyle() directly... dunno if this is helpful but i was just reading someone on stackoverflow calling the use of that method directly a "react anti-pattern"

@michelcrypt4d4mus
Copy link

i've only been a react-native dev for like 3 days but one thing that seems like it might be relevant here is that react-native does not use <div> as a container class... instead there is a <View> component.

while i have done v. little web/react development in my life i have a hunch that getComputedStyle() is barfing bc it's not finding <div> containers etc. and probably finding a bunch of components it has never seen before bc it's outside of a browser setting.

@CarlosNZ
Copy link
Owner Author

i've only been a react-native dev for like 3 days but one thing that seems like it might be relevant here is that react-native does not use <div> as a container class... instead there is a <View> component.

I actually think it probably can't use 0.5s at all, as it's a browser window object method. I can see that I can quite easily avoid using it, but now I wonder how many other browser-specific methods I'm using that might fail on React Native.

json-edit-react-v1.9.3-0.tgz

Okay, I've just make a quick build where I've replaced the getComputedStyle call with a hard-coded value. Can you try it out and see if anything else is breaking? (You should be able to import it with npm locally, i.e. npm install ~/path/to/tarball)

If there's too many problems, I may have to make non-browser support a "coming soon" feature for now 😔

@michelcrypt4d4mus
Copy link

i'm happy to try it out but how do i install a node package from a zip file? sorry i'm new here in javascript world.

@CarlosNZ
Copy link
Owner Author

i'm happy to try it out but how do i install a node package from a zip file? sorry i'm new here in javascript world.

Just download it and use npm as usual, but with a path to the file (don't unzip it, it's in .tar format, which is what npm downloads when you npm install normally).

So npm install /localPath/folder/whatever/json-edit-react-v1.9.3-0.tgz

@michelcrypt4d4mus
Copy link

ok i tried it and it went poorly but in a different way than it was going poorly before:

 ERROR  Invariant Violation: View config getter callback for component `path` must be a function (received `undefined`). Make sure to start component names with a capital letter.

This error is located at:
    in path (created by IconChevron)
    in svg (created by IconChevron)
    in IconChevron (created by Icon)
    in Icon (created by CollectionNode)
    in div (created by CollectionNode)
    in div (created by CollectionNode)
    in div (created by CollectionNode)
    in div (created by CollectionNode)
    in CollectionNode (created by Editor)
    in div (created by Editor)
    in Editor
    in CollapseProvider
    in ThemeProvider
    in Unknown (at current_daemon_status.js:32)
    in CurrentDaemonStatus (at App.js:20)
    in RCTView (at View.js:116)
    in View (at ImageBackground.js:82)
    in ImageBackground (at App.js:17)
    in RCTView (at View.js:116)
    in View (at App.js:15)
    in Provider (at App.js:14)
    in App (at withDevTools.ios.js:29)
    in withDevTools(App) (at renderApplication.js:57)
    in RCTView (at View.js:116)
    in View (at AppContainer.js:127)
    in RCTView (at View.js:116)
    in View (at AppContainer.js:155)
    in AppContainer (at renderApplication.js:50)
    in main(RootComponent) (at renderApplication.js:67), js engine: hermes
 WARN  Excessive number of pending callbacks: 501. Some pending callbacks that might have leaked by never being called from native code: {"1183":{},"1185":{},"1187":{},"1189":{},"1191":{},"1193":{},"1195":{},"1197":{},"1199":{},"1201":{},"1203":{},"1205":{},"1207":{},"1209":{},"1211":{},"1213":{},"1215":{},"1217":{},"1219":{},"1221":{},"1223":{},"1225":{},"1227":{},"1229":{},"1231":{},"1233":{},"1235":{},"1237":{},"1239":{},"1241":{},"1243":{},"1245":{},"1247":{},"1249":{},"1251":{},"1253":{},"1255":{},"1257":{},"1259":{},"1261":{},"1263":{},"1265":{},"1267":{},"1269":{},"1271":{},"1273":{},"1275":{},"1277":{},"1279":{},"1281":{},"...(truncated keys)...":451}

@CarlosNZ
Copy link
Owner Author

Okay, I'll have to have a proper look at it sometime. I guess we can say it doesn't support React Native for now. Sorry about that.

But thanks for bringing it to my attention, I'll definitely work on soon.

@CarlosNZ CarlosNZ added the enhancement New feature or request label May 10, 2024
@CarlosNZ CarlosNZ changed the title Error: Property 'getComputedStyle' doesn't exit Support React Native May 10, 2024
@michelcrypt4d4mus
Copy link

np thanks for looking into it so quickly!

@michelcrypt4d4mus
Copy link

i was poking around this repo a bit wondering how hard it would be to make it react-native compatible and i found this discussion of what changes need to be made. on the plus side i don't think yr code involves much in the way of routing but on the negative side i'm not sure there's a way to do it other than replacing every <div> with a <View> and every <p> with a <Text> and a couple other things... kind of tempting to see if i can just sed my way to victory though i'm also experienced enough to know that it will probably end in tears.

@CarlosNZ CarlosNZ added user request Requested by user and removed bug Something isn't working labels May 18, 2024
@michelcrypt4d4mus
Copy link

just here to report back after getting up to speed on React Native (and thus the differences between it and regular React):

  • for the most part i think you could use simple sed to just replace elements:
    1. div -> View
    2. textarea -> TextInput
    3. span -> Text
    4. a -> Text with onPress=() => Linking.openUrl(urlString) attribute
    5. img -> Image
    6. svg -> react-native-svg
  • where it would get a bit hinky would be with CSS. the number of basic styling CSS options that work in React but don't work in ReactNative is vanishingly small but anything that involves CSS animation either doesn't work or works a completely different way (often not through CSS)

as my use case has grown up it turns our json-edit-react wouldn't be a great fit but just figured i'd share what i learned in case you decide to go through with this feature.

@CarlosNZ
Copy link
Owner Author

CarlosNZ commented Jun 10, 2024

Thanks for that info @michelcrypt4d4mus . I think it'll probably be on my "longer term" roadmap, but we'll see if there's much demand.

@CarlosNZ
Copy link
Owner Author

If anyone else wanted to look further into this and perhaps take it on, please let me know.

I wonder if it would end up being different enough that it would need to be a separate package?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request user request Requested by user
Projects
None yet
Development

No branches or pull requests

2 participants