[native] Omit extensions when importing components with platform-specific extensions
ClosedPublic
Actions

Authored by ashoat on Feb 13 2023, 5:13 PM.

Details

Reviewers

atul
tomek

Commits

rCOMM0e2804a3c8ca: [native] Omit extensions when importing components with platform-specific…

Summary

We use this functionality of Metro (React Native packager) to fork code between platforms: link

My recent work to add file extensions to all imports broke this functionality. Metro does this mapping by suffixing eg. .ios.js to the import path, so when we include .js in the import that makes it impossible for things to work.

This diff reverts the changes in D6692 for the subset of files in native where we use platform-specific extensions, and adds an eslint-ignore to disable the check from D6694.

Test Plan

Add a console.log to one of the iOS files and confirm the log prints in the iOS simulator (it didn't before this diff)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Feb 13 2023, 5:13 PM

Harbormaster completed remote builds in B16470: Diff 22516.Feb 13 2023, 5:27 PM

ashoat requested review of this revision.Feb 13 2023, 5:27 PM

Is there a better solution for this? The fact that we have to be aware that an import has different variants might cause maintainability issues in the future.

I'm also curious how it works currently. If one file has .ios.js extension and one has .js and we import without any extension, the extension on Android is automatically added. So it seems like skipping the extension sometimes works.

This revision is now accepted and ready to land.Feb 14 2023, 4:24 AM

In D6729#200280, @tomek wrote:

Is there a better solution for this? The fact that we have to be aware that an import has different variants might cause maintainability issues in the future.

I'm also curious how it works currently. If one file has .ios.js extension and one has .js and we import without any extension, the extension on Android is automatically added. So it seems like skipping the extension sometimes works.

I don't think there's a better solution. Here's the error we see if Metro can't find a file:

Screenshot 2023-02-14 at 8.12.55 AM.png (2×1 px, 971 KB)

You can see how it's looking for the file. It tries three things: opening the file directly, appending extensions, and then treating the file like a directory and looking for an index.js.

If we try to import a file with the .js extension in the import, then there is no way for the platform-specific extensions to work, as they happen by appending the extensions.

An alternative would be to use Platform and manually determine which component to use, but I don't think it's worth it.

Closed by commit rCOMM0e2804a3c8ca: [native] Omit extensions when importing components with platform-specific… (authored by ashoat). · Explain WhyFeb 14 2023, 5:16 AM

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMM0e2804a3c8ca: [native] Omit extensions when importing components with platform-specific….