Support Webpack compilation

[Nickersoft]

Description

This PR simply changes the output format for JSX from react-native to react-jsx, as the former results in unprocessed JSX that transpilers like Webpack don't understand. Confirmed to work on web and iOS.

Fixes # (issue)

#346

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Tested manually using Storybook for web and iOS.

[changeset-bot]

🦋 Changeset detected

Latest commit: 9fa0ef4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
victory-native	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
victory-native-xl-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 12, 2024 5:02am

[zibs]

Thanks, it sounds like this should be okay. I took this for a spin on iOS and Android and it looks good to me -- @carbonrobot confirm?

[carbonrobot]

@zibs @Nickersoft The react-native option was originally used to support consumers that used the Metro bundler (Expo), so we just need build a local package and confirm it still works in an expo project for iOS and Android.

[carbonrobot]

Looks like it requires the following versions

React 17 RC and higher supports it, but we’ve also released React 16.14.0, React 15.7.0, and React 0.14.10

So we would have to be comfortable dropping support for anything outside those ranges as well. Looking at the npmjs stats, 16.14 seems like a reasonable cutoff, so I would be ok with that. It's the oldest release with over 1mil+ weekly downloads. Everything under that is significantly less.

[Nickersoft]

@carbonrobot If it's any consolidation, I've been using react-jsx successfully via Expo for the Storybook app I'm running!

[carbonrobot]

@Nickersoft Awesome! As far as I can tell, its supported for the last couple versions, I sent out a message to someone on the Expo team for confirmation, but I feel pretty good about it.

[zibs]

Okay, I was able to test this as lib/dist build locally against iOS and Android and it seems like it worked, so I think this is good.

However, @Nickersoft can you clean up this PR so that it's really only the single line change we need here?

[Nickersoft]

Awesome @zibs, thanks! Cleaned it up.

So in addition to the tsconfig change, after using this fork in my project, I realized I had to make another fix too in lib/src/cartesian/hooks/useChartPressState.ts to explicitly pass hook dependencies to the Reanimated hook, as the Reanimated babel plugin in my project wouldn't be applied seeing this library is now being treated as a pre-compiled, external dependency. Nothing else seemed to break, but I do wonder if there are other areas of the code that might be affected in a similar vein.

Curious if there's a way the code could be compiled using that Babel plugin to inject the dependencies as part of this repo's build step? For reference, it's this one: https://docs.swmansion.com/react-native-reanimated/docs/fundamentals/getting-started/#step-2-add-reanimateds-babel-plugin. Otherwise, dependencies need to be explicitly specified for all animation-related hooks in the project or they may not work on web.

[Nantris]

Friendly bump. Is there any way to use victory-native-xl in a cross-platform manner at this time?

[Nickersoft]

@zibs is this good to merge?

[carbonrobot]

@Nickersoft Thanks for the cleanup. Just to confirm, are you saying the missing hook dependency is required for Webpack compilation, or is that part of a separate bug fix?

[Nickersoft]

Yes it is @carbonrobot – in fact, I fear this may not be the only place it's required (though it's the only bug I caught). Basically by precompiling the code for web projects, downstream users aren't required to use Reanimated's Babel plugin that would typically automatically add these dependencies. In fact, most web bundlers won't transpile any source code from NPM packages like they might for a RN project. So we'd probably want to make sure we're accurately specifying Reanimated's dependencies in these hooks, though I'm not sure how widespread the issue is.

[carbonrobot]

I don't think we are yet capturing all the requirements in their documentation at this point in order to support web builds.

If there are other unknown bugs, we would also need to verify and correct those before this is mergeable.

There is a guide here on implementing this without using Babel by doing the following:

• Update our docs to indicate users will need the @babel/plugin-transform-class-properties installed in their Babel configuration to support web builds (note: the docs incorrectly state the deprecated @babel/plugin-proposal-class-properties)
• Add an eslint rule to catch any code that is not correctly setup with dependencies for react-native-reanimated hooks
• Update all affected hooks