Skip to content

fix: findNodeHandle on the web platform #912

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

Merged

Conversation

IvanIhnatsiuk
Copy link
Contributor

📜 Description

In the new version of react-native-web they added an error throw if you try to use findNodeHandle from react-native https://github.com/necolas/react-native-web/blob/922c134f2b7c428cc19daaeb3cac4b6a4c8ec6a3/packages/react-native-web/src/exports/findNodeHandle/index.js#L11. So I created a findNodeHandle utility that uses react-native's findNodeHandle for the native platform and returns Node or null for web.

💡 Motivation and Context

This library should work well on all platforms

📢 Changelog

JS

  • added findNodeHandle util

🤔 How Has This Been Tested?

tested in a web browser

📸 Screenshots (if appropriate):

Before:

Screen.Recording.2025-04-10.at.11.20.20.mov

After:

Screen.Recording.2025-04-10.at.11.19.08.mov

📝 Checklist

  • CI successfully passed
  • I added new mocks and corresponding unit-tests if library API was changed

Copy link
Contributor

📊 Package size report

Current size Target Size Difference
177281 bytes 176200 bytes 1081 bytes 📈

@IvanIhnatsiuk IvanIhnatsiuk changed the title fix: findNodeHandle on web platform fix: findNodeHandle on the web platform Apr 10, 2025
@kirillzyusko kirillzyusko self-assigned this Apr 10, 2025
@kirillzyusko kirillzyusko added the 🌐 web Web specific label Apr 10, 2025
@kirillzyusko kirillzyusko added the 🎯 crash Library triggers a crash of the app label Apr 17, 2025
@kirillzyusko
Copy link
Owner

Since I don't support web - it's okay to return element as node-handle.

On web for registerEventHandler I use mock, KeyboardAwareScrollView also will work with these changes (it will compare node-handle with null, so will be false and will not work on web (and it's expected)).

@kirillzyusko kirillzyusko merged commit 0071eb0 into kirillzyusko:main Apr 17, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎯 crash Library triggers a crash of the app 🌐 web Web specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants