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

Replace metro-react-native-babel-preset with @react-native/babel-preset #573

Merged
merged 2 commits into from
Feb 9, 2024

Conversation

dmytrorykun
Copy link
Contributor

Summary:

In React Native 0.73 metro-react-native-babel-preset has been moved from Metro to React Native repo and renamed to @react-native/babel-preset. See https://reactnative.dev/blog/2023/12/06/0.73-debugging-improvements-stable-symlinks#babel-package-renames

It is important to update this dependency because it makes the library compatible with the New Architecture.

Test Plan:

cd package
yarn prepare
cat dist/RNCSliderNativeComponent.js

Before:

var _interopRequireDefault=require("@babel/runtime/helpers/interopRequireDefault");Object.defineProperty(exports,"__esModule",{value:true});exports.default=void 0;var _codegenNativeComponent=_interopRequireDefault(require("react-native/Libraries/Utilities/codegenNativeComponent"));var _default=exports.default=(0,_codegenNativeComponent.default)('RNCSlider');% 

After:

var _interopRequireDefault=require("@babel/runtime/helpers/interopRequireDefault");Object.defineProperty(exports,"__esModule",{value:true});exports.default=exports.__INTERNAL_VIEW_CONFIG=void 0;var _codegenNativeComponent=_interopRequireDefault(require("react-native/Libraries/Utilities/codegenNativeComponent"));var NativeComponentRegistry=require('react-native/Libraries/NativeComponent/NativeComponentRegistry');var _require=require('react-native/Libraries/NativeComponent/ViewConfigIgnore'),ConditionallyIgnoredEventHandlers=_require.ConditionallyIgnoredEventHandlers;var nativeComponentName='RNCSlider';var __INTERNAL_VIEW_CONFIG={uiViewClassName:'RNCSlider',bubblingEventTypes:{topChange:{phasedRegistrationNames:{captured:'onChangeCapture',bubbled:'onChange'}},topRNCSliderValueChange:{phasedRegistrationNames:{captured:'onRNCSliderValueChangeCapture',bubbled:'onRNCSliderValueChange'}}},directEventTypes:{topRNCSliderSlidingStart:{registrationName:'onRNCSliderSlidingStart'},topRNCSliderSlidingComplete:{registrationName:'onRNCSliderSlidingComplete'}},validAttributes:Object.assign({accessibilityUnits:true,accessibilityIncrements:true,disabled:true,inverted:true,vertical:true,tapToSeek:true,maximumTrackImage:{process:require('react-native/Libraries/Image/resolveAssetSource')},maximumTrackTintColor:{process:require('react-native/Libraries/StyleSheet/processColor').default},maximumValue:true,minimumTrackImage:{process:require('react-native/Libraries/Image/resolveAssetSource')},minimumTrackTintColor:{process:require('react-native/Libraries/StyleSheet/processColor').default},minimumValue:true,step:true,testID:true,thumbImage:{process:require('react-native/Libraries/Image/resolveAssetSource')},thumbTintColor:{process:require('react-native/Libraries/StyleSheet/processColor').default},trackImage:{process:require('react-native/Libraries/Image/resolveAssetSource')},value:true,lowerLimit:true,upperLimit:true},ConditionallyIgnoredEventHandlers({onChange:true,onRNCSliderSlidingStart:true,onRNCSliderSlidingComplete:true,onRNCSliderValueChange:true}))};exports.__INTERNAL_VIEW_CONFIG=__INTERNAL_VIEW_CONFIG;var _default=NativeComponentRegistry.get(nativeComponentName,function(){return __INTERNAL_VIEW_CONFIG;});exports.default=_default;% 

Pay attention that the after output contains the __INTERNAL_VIEW_CONFIG objects, they are crucial for the New Architecture.

@vafada
Copy link
Contributor

vafada commented Jan 31, 2024

you missed commiting your change for example/package.json ?

https://github.com/callstack/react-native-slider/blob/main/example/package.json#L41

and

https://github.com/callstack/react-native-slider/blob/main/example/babel.config.json

@dmytrorykun
Copy link
Contributor Author

@BartoszKlonowski could you please review this?

Copy link
Member

@BartoszKlonowski BartoszKlonowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Thank you for the contribution, @dmytrorykun!

@BartoszKlonowski BartoszKlonowski merged commit 463a031 into callstack:main Feb 9, 2024
8 checks passed
@dmytrorykun
Copy link
Contributor Author

@BartoszKlonowski thank you! Please note that you need to publish the new version of the package for this change to take the effect. Are you planning to publish the new version any time soon?

@BartoszKlonowski
Copy link
Member

I can't say it will be soon, because at the same time the custom step marker is going to be published, so before that everything needs to be polished for a safe release, and that's going to take a bit.

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.

None yet

3 participants