feat(web): basic support for react-native-web #369
Conversation
one works on all platforms, one only works android/ios
empty now, but lays the groundwork for real features
used the file extension override mechanism for web-specific module so it loads this vs the current main module which does not work on web
mikehardy
force-pushed
the
@mikehardy/react-native-web
branch
from
a8c98c0
to
49d98a4
Compare
…) has default returns this focuses the existing native calls to the platforms they really work on, and has a default return with dummy values for all other platforms. This defintely works for react-native-web and theoretically means we won't break on windows or macos either
it should be possible to build the javascript source from the installed package, this can encourage casual contributions via patch-package
mikehardy
force-pushed
the
@mikehardy/react-native-web
branch
from
49d98a4
to
c852637
Compare
Codecov Report
@@ Coverage Diff @@
## main #369 +/- ##
==========================================
- Coverage 78.65% 77.41% -1.24%
==========================================
Files 29 30 +1
Lines 1564 1602 +38
Branches 520 534 +14
==========================================
+ Hits 1230 1240 +10
- Misses 284 311 +27
- Partials 50 51 +1 |
| return Promise.resolve({ | ||
| manufacturer: 'apple', |
There was a problem hiding this comment.
@helenaford this is the only line that is not 100% backwards compatible. I think we should just put the platform in here so I implemented it as such, otherwise if we want 100% backwards compatibility on the return value (that no one is probably using?) we need to special case Platform.OS === ios to do it
There was a problem hiding this comment.
@mikehardy mmm interesting, yeah i looked and it's for the getPowerManagerInfo endpoint... probably OK like you say 😅
I was slightly nervous seeing this as I have a big flutter p/r coming, but the changes look good. Only changes under packages/react-native 😎 nice!
There was a problem hiding this comment.
Yep - adding react-native-web support is actually really easy (well, stubs at least). If I tried to post my tvOS one again that might perturb things, no plans for that at the moment.... Flutter PR, nice!
|
Thanks for the review @yamankatby ! This sets us up with the structure needed for any react-native-web functionality you want to add. For any new web feature just implement it in NotifeeNativeModule.web.ts and in NotifeeApiModule.ts simply start sending the calls through to the new implementation. I found it really difficult to test this stuff which is why I made an out-of-tree throw-away test using luna template (https://github.com/criszz77/luna) but we should definitely get our example app set up with some web scaffolding in the future. @helenaford any chance of a merge+release with this 🙏 :-), then I can start actually integrating it in work apps to verify it really is good to go like I think it is. If so please rebase-merge - I kept the commits squeaky clean so each one makes sense on it's own |
There are no actual APIs productively implemented, however the structure for react-native-web support is in place now, and it builds correctly and runs on react-native-web without failures, so notifee may safely be included in react-native-web projects
Demonstration:
mikehardy/rnfbdemo@d0f7ef9
Clean console from
yarn webinnotifeewebdemoon that commit of my demo script:As discussed in #364 with @thecompoundingdev as well as on Slack with @yamankatby and @Ehesp