Skip to content

Commit

Permalink
refact: remove mixed CJS/ESM, refactorize index.native.tsx (#1982)
Browse files Browse the repository at this point in the history
## Description

Users are reporting that on Vite they cannot build their project with
react-native-screens because of the mixed CJS/ESM files that are being
created while building a bundle (you can see
[here](https://publint.dev/[email protected]) that some
package managers reports errors about mixed CJS/ESM files). To reduce
that behavior I've decided to remove CJS completely while bundling the
project, resulting in building only ESM files. Unfortunately because of
that I had to remove optional requiring process inside the
index.native.tsx file, but this shouldn't have a large impact while
using rn-screens.

I also decided to move some parts of the screens implementation to
separate files - this should improve readability, better understanding
of code for newcomers and should improve developer experience overall.
Having only imports and exports in index files is also a good practice -
this was my main reason why I've planned to do that.

Closes #1908 - I'll try to ensure that this will fix Vite for sure 👍 

## Changes

- Disable bundling CJS files from react-native-screens
- Refactorize index.native.tsx files to separate files

## Test code and steps to reproduce

First, try to bundle the project - you can see that inside `lib` there
shouldn't be `common` directory with the CJS files.
Then, try to run FabricTestExample with a couple of tests. Application
should work properly as usual.

## Developer notes
There are some points that I stumbled upon and I need to mention here.
- I've managed to move all of the native components from class to
function components, **except**:
- **Screen:** Unfortunately we need to stay with class components there,
as for now we would like to keep behavior of using `setNativeProps` for
a screen (does anybody do that? Or is react-native calling this method
for a screen wherever? There's a field for a discussion).
- **SearchBar:** Because of managing ref's state and dropping it down to
the methods responsible for commands I was also unable to convert this
to functional component.

- I tried to also refactor index.tsx file, but I see no reason to do
this. For now I'm keeping it as it is (with only a slight changes to
this file):
- Because of a conflict of naming between SearchBarCommands (from
types.tsx) and SearchBarCommands as a native component -> it's not that
easy to fix, so I suggest fixing this in a future (might be also a good
first issue).
- I also tried to move `index.native.tsx` to `index.tsx` and to move
`index.tsx` to `index.web.tsx`, but because of a conflict I described
above and because I don't see the point of rendering conditionally
native components depending if `Platform.OS !== 'web'` (and rendering a
`View` if Platform.OS is web) I'm keeping the current naming.
- Let me know what do you think about the refactor of index.native.tsx!
This change is a **Proof of concept** and I codenamed it as a second
stage of this PR, so we might give it a try, but I'm all ears about your
opinion - IMO it is worth merging :shipit:.

## Checklist

- [X] Ensured that nothing changed while refactoring index.native.tsx
- [X] Ensured that CI passes
  • Loading branch information
tboba authored Jan 18, 2024
1 parent d28a1f7 commit 3602dad
Show file tree
Hide file tree
Showing 25 changed files with 1,468 additions and 1,268 deletions.
2 changes: 1 addition & 1 deletion native-stack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"main": "../lib/commonjs/native-stack/index",
"module": "../lib/module/native-stack/index",
"react-native": "../src/native-stack/index",
"types": "../lib/typescript/native-stack/index"
"types": "../lib/typescript/native-stack/index.d.ts"
}
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"module": "lib/module/index",
"react-native": "src/index",
"source": "src/index",
"types": "lib/typescript/index",
"types": "lib/typescript/index.d.ts",
"files": [
"src/",
"common/",
Expand Down Expand Up @@ -71,7 +71,6 @@
"devDependencies": {
"@babel/core": "^7.20.0",
"@babel/eslint-parser": "7.22.15",
"@react-native-community/bob": "^0.17.1",
"@react-native-community/cli": "^11.3.6",
"@react-native-community/cli-platform-android": "^11.3.6",
"@react-native-community/cli-platform-ios": "^11.3.6",
Expand Down Expand Up @@ -102,6 +101,7 @@
"react": "18.2.0",
"react-dom": "^18.2.0",
"react-native": "0.72.4",
"react-native-builder-bob": "^0.23.2",
"react-native-gesture-handler": "^2.13.3",
"react-native-reanimated": "3.7.0-nightly-20240109-9e2c33716",
"react-native-safe-area-context": "^4.8.1",
Expand All @@ -122,7 +122,7 @@
"android/**/*.kt": "yarn format-android",
"ios/**/*.{h,m,mm,cpp}": "yarn format-ios"
},
"@react-native-community/bob": {
"react-native-builder-bob": {
"source": "src",
"output": "lib",
"targets": [
Expand Down
2 changes: 1 addition & 1 deletion reanimated/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"main": "../lib/commonjs/reanimated/index",
"module": "../lib/module/reanimated/index",
"react-native": "../src/reanimated/index",
"types": "../lib/typescript/reanimated/index"
"types": "../lib/typescript/reanimated/index.d.ts"
}
25 changes: 25 additions & 0 deletions src/components/FullWindowOverlay.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import React, { PropsWithChildren, ReactNode } from 'react';
import { Platform, StyleProp, View, ViewStyle } from 'react-native';

// Native components
import FullWindowOverlayNativeComponent from '../fabric/FullWindowOverlayNativeComponent';
const NativeFullWindowOverlay: React.ComponentType<
PropsWithChildren<{
style: StyleProp<ViewStyle>;
}>
> = FullWindowOverlayNativeComponent as any;

Check warning on line 10 in src/components/FullWindowOverlay.tsx

View workflow job for this annotation

GitHub Actions / install-and-lint

Unexpected any. Specify a different type

function FullWindowOverlay(props: { children: ReactNode }) {
if (Platform.OS !== 'ios') {
console.warn('Using FullWindowOverlay is only valid on iOS devices.');
return <View {...props} />;
}
return (
<NativeFullWindowOverlay
style={{ position: 'absolute', width: '100%', height: '100%' }}>
{props.children}
</NativeFullWindowOverlay>
);
}

export default FullWindowOverlay;
6 changes: 6 additions & 0 deletions src/components/FullWindowOverlay.web.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { View } from 'react-native';
import React, { ReactNode } from 'react';

export default View as React.ComponentType<{
children: ReactNode;
}>;
194 changes: 194 additions & 0 deletions src/components/Screen.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
/* eslint-disable @typescript-eslint/no-var-requires */
import React from 'react';
import { Animated, View } from 'react-native';

import TransitionProgressContext from '../TransitionProgressContext';
import DelayedFreeze from './helpers/DelayedFreeze';
import { ScreenProps } from 'react-native-screens';

import {
freezeEnabled,
isNativePlatformSupported,
screensEnabled,
} from '../core';

// Native components
import ScreenNativeComponent from '../fabric/ScreenNativeComponent';

export const NativeScreen: React.ComponentType<ScreenProps> =
ScreenNativeComponent as any;

Check warning on line 19 in src/components/Screen.tsx

View workflow job for this annotation

GitHub Actions / install-and-lint

Unexpected any. Specify a different type
let AnimatedNativeScreen: React.ComponentType<ScreenProps>;

// Incomplete type, all accessible properties available at:
// react-native/Libraries/Components/View/ReactNativeViewViewConfig.js
interface ViewConfig extends View {
viewConfig: {
validAttributes: {
style: {
display: boolean;
};
};
};
}

export class InnerScreen extends React.Component<ScreenProps> {
private ref: React.ElementRef<typeof View> | null = null;
private closing = new Animated.Value(0);
private progress = new Animated.Value(0);
private goingForward = new Animated.Value(0);

setNativeProps(props: ScreenProps): void {
this.ref?.setNativeProps(props);
}

setRef = (ref: React.ElementRef<typeof View> | null): void => {
this.ref = ref;
this.props.onComponentRef?.(ref);
};

render() {
const {
enabled = screensEnabled(),
freezeOnBlur = freezeEnabled(),
...rest
} = this.props;

// To maintain default behavior of formSheet stack presentation style and to have reasonable
// defaults for new medium-detent iOS API we need to set defaults here
const {
sheetAllowedDetents = 'large',
sheetLargestUndimmedDetent = 'all',
sheetGrabberVisible = false,
sheetCornerRadius = -1.0,
sheetExpandsWhenScrolledToEdge = true,
} = rest;

if (enabled && isNativePlatformSupported) {
AnimatedNativeScreen =
AnimatedNativeScreen || Animated.createAnimatedComponent(NativeScreen);

let {
// Filter out active prop in this case because it is unused and
// can cause problems depending on react-native version:
// https://github.com/react-navigation/react-navigation/issues/4886
active,
activityState,
children,
isNativeStack,
gestureResponseDistance,
onGestureCancel,
...props
} = rest;

if (active !== undefined && activityState === undefined) {
console.warn(
'It appears that you are using old version of react-navigation library. Please update @react-navigation/bottom-tabs, @react-navigation/stack and @react-navigation/drawer to version 5.10.0 or above to take full advantage of new functionality added to react-native-screens'
);
activityState = active !== 0 ? 2 : 0; // in the new version, we need one of the screens to have value of 2 after the transition
}

const handleRef = (ref: ViewConfig) => {
if (ref?.viewConfig?.validAttributes?.style) {
ref.viewConfig.validAttributes.style = {
...ref.viewConfig.validAttributes.style,
display: false,
};
this.setRef(ref);
}
};

return (
<DelayedFreeze freeze={freezeOnBlur && activityState === 0}>
<AnimatedNativeScreen
{...props}
activityState={activityState}
sheetAllowedDetents={sheetAllowedDetents}
sheetLargestUndimmedDetent={sheetLargestUndimmedDetent}
sheetGrabberVisible={sheetGrabberVisible}
sheetCornerRadius={sheetCornerRadius}
sheetExpandsWhenScrolledToEdge={sheetExpandsWhenScrolledToEdge}
gestureResponseDistance={{
start: gestureResponseDistance?.start ?? -1,
end: gestureResponseDistance?.end ?? -1,
top: gestureResponseDistance?.top ?? -1,
bottom: gestureResponseDistance?.bottom ?? -1,
}}
// This prevents showing blank screen when navigating between multiple screens with freezing
// https://github.com/software-mansion/react-native-screens/pull/1208
ref={handleRef}
onTransitionProgress={
!isNativeStack
? undefined
: Animated.event(
[
{
nativeEvent: {
progress: this.progress,
closing: this.closing,
goingForward: this.goingForward,
},
},
],
{ useNativeDriver: true }
)
}
onGestureCancel={
onGestureCancel ??
(() => {
// for internal use
})
}>
{!isNativeStack ? ( // see comment of this prop in types.tsx for information why it is needed
children
) : (
<TransitionProgressContext.Provider
value={{
progress: this.progress,
closing: this.closing,
goingForward: this.goingForward,
}}>
{children}
</TransitionProgressContext.Provider>
)}
</AnimatedNativeScreen>
</DelayedFreeze>
);
} else {
// same reason as above
let {
active,
activityState,
style,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
onComponentRef,
...props
} = rest;

if (active !== undefined && activityState === undefined) {
activityState = active !== 0 ? 2 : 0;
}
return (
<Animated.View
style={[style, { display: activityState !== 0 ? 'flex' : 'none' }]}
ref={this.setRef}
{...props}
/>
);
}
}
}

// context to be used when the user wants to use enhanced implementation
// e.g. to use `useReanimatedTransitionProgress` (see `reanimated` folder in repo)
export const ScreenContext = React.createContext(InnerScreen);

class Screen extends React.Component<ScreenProps> {
static contextType = ScreenContext;

render() {
const ScreenWrapper = (this.context || InnerScreen) as React.ElementType;
return <ScreenWrapper {...this.props} />;
}
}

export default Screen;
43 changes: 43 additions & 0 deletions src/components/Screen.web.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { ScreenProps } from 'react-native-screens';
import { Animated, View } from 'react-native';
import React from 'react';

import { screensEnabled } from '../core';

export const InnerScreen = View;

// We're using class component here because of the error from reanimated:
// createAnimatedComponent` does not support stateless functional components; use a class component instead.
export class NativeScreen extends React.Component<ScreenProps> {
render(): JSX.Element {
let {
active,
activityState,
style,
enabled = screensEnabled(),
...rest
} = this.props;

if (enabled) {
if (active !== undefined && activityState === undefined) {
activityState = active !== 0 ? 2 : 0; // change taken from index.native.tsx
}
return (
<View
// @ts-expect-error: hidden exists on web, but not in React Native
hidden={activityState === 0}
style={[style, { display: activityState !== 0 ? 'flex' : 'none' }]}
{...rest}
/>
);
}

return <View {...rest} />;
}
}

const Screen = Animated.createAnimatedComponent(NativeScreen);

export const ScreenContext = React.createContext(Screen);

export default Screen;
33 changes: 33 additions & 0 deletions src/components/ScreenContainer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { Platform, View } from 'react-native';
import React from 'react';
import { ScreenContainerProps } from 'react-native-screens';
import { isNativePlatformSupported, screensEnabled } from '../core';

// Native components
import ScreenContainerNativeComponent from '../fabric/ScreenContainerNativeComponent';
import ScreenNavigationContainerNativeComponent from '../fabric/ScreenNavigationContainerNativeComponent';

export const NativeScreenContainer: React.ComponentType<ScreenContainerProps> =
Platform.OS !== 'web' ? (ScreenContainerNativeComponent as any) : View;

Check warning on line 11 in src/components/ScreenContainer.tsx

View workflow job for this annotation

GitHub Actions / install-and-lint

Unexpected any. Specify a different type
export const NativeScreenNavigationContainer: React.ComponentType<ScreenContainerProps> =
Platform.OS !== 'web'
? (ScreenNavigationContainerNativeComponent as any)

Check warning on line 14 in src/components/ScreenContainer.tsx

View workflow job for this annotation

GitHub Actions / install-and-lint

Unexpected any. Specify a different type
: View;

function ScreenContainer(props: ScreenContainerProps) {
const { enabled = screensEnabled(), hasTwoStates, ...rest } = props;

if (enabled && isNativePlatformSupported) {
if (hasTwoStates) {
const ScreenNavigationContainer =
Platform.OS === 'ios'
? NativeScreenNavigationContainer
: NativeScreenContainer;
return <ScreenNavigationContainer {...rest} />;
}
return <NativeScreenContainer {...rest} />;
}
return <View {...rest} />;
}

export default ScreenContainer;
6 changes: 6 additions & 0 deletions src/components/ScreenContainer.web.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { View } from 'react-native';

export const NativeScreenContainer = View;
export const NativeScreenNavigationContainer = View;

export default View;
Loading

0 comments on commit 3602dad

Please sign in to comment.