[MOO-2401]: add mendix-native as dependency - #613
Conversation
|
|
||
| // V3 dropped the feature of providing an action sheet so users can decide on which action to take, camera or library. | ||
| const nativeVersionMajor = NativeModules?.ImagePickerManager?.showImagePicker ? 2 : 4; | ||
| const nativeVersionMajor = getNativeModule<{ showImagePicker?: unknown }>("ImagePickerManager")?.showImagePicker |
There was a problem hiding this comment.
I would consider having the ImagePickerManager extracted in a separate scoped module under mendix-native, similar to file-system.
Since we are using it in more than 2 places here. It would be cleaner and more maintainable to end up with a simple
import { ImagePickerManager } from ....
const nativeVersionMajor = ImagePickerManager.version;
There was a problem hiding this comment.
Following Yogendra's more detailed review here, this suggestion of mine might bring more maintenance hassle in the future.
The separation in files might still be valid though, but probably not as part of mendix-native repo. Better to discuss this with @YogendraShelke as a single source of truth 😅 .
| jest.mock("react-native", () => { | ||
| const RN = jest.requireActual("react-native"); | ||
| RN.NativeModules.RNCWebView = { isFileUploadSupported: jest.fn(() => true) }; | ||
| const nativeModules = RN["Native" + "Modules"]; |
There was a problem hiding this comment.
This access via string concatenation looks redundant. Is there a reason why we are not keeping the direct access via RN.NativeModules?
| // BEGIN USER CODE | ||
| // Documentation https://github.com/invertase/notifee | ||
| if (NativeModules && !NativeModules.NotifeeApiModule) { | ||
| if (!getNativeModule("NotifeeApiModule")) { |
There was a problem hiding this comment.
Similar to the comment above , let's consider if having a separate declaration for this module in mendix-native would be better.
So we don't use a generic API to 'manually type and search' for modules, but rely on the exposed imports of mendix-native/*.
This affects the other statically typed modules below as well e.g. RNFBMessagingModule.
d08d309 to
7083f68
Compare
3 participants
Checklist
[XX-000]: description)? ✅ ❌Feature specific
Please remove unnecessary emojis and sections and this comment before proceeding
This PR contains
What is the purpose of this PR?
...
Relevant changes
Please add a high level explanation of what was changed and how the initial problem was solved
What should be covered while testing?
...
Extra comments (optional)
Please add extra comments or delete the section if not required