Move macOS JS module overrides to their own files #1961
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a sequence of changes to move macOS files to *.macos.js files, hopefully without losing history and to show that we're not making explicit changes to macOS files. It will use a diamond pattern, where any file that was forked for macOS will be moved from it's source file first and merged with a commit that moves a file back to it's original file location. Here are the steps: 1. Move all forked JS files to *.macos.js 2. Merge in a branch that moves the forked files to *.native.js files and then back to their original file
We cannot convince git history to treat an overwrite of Image.macos.js with Image.ios.js unless the file is deleted first.
In order to preserve history of Image.ios.js, we move the file to Image.macos.js, rather than just overwriting it in the first place.
We can retain the same history for both the original and the copied file if we copy it twice and merge the commits. This copies the files to a different location to copy them back.
|
Before merging this PR, we should probably resolve the following:
|
rozele
force-pushed
the
macosJsOverrides
branch
from
d060c76
to
78e69a5
Compare
rozele
force-pushed
the
macosJsOverrides
branch
from
78e69a5
to
f1228d9
Compare
Since the iOS code no longer has macOS specific logic in it, this is the result of running `yarn test-ci -u`. Additionally, because `@react-native/virtualized-lists` is not part of the react-native-macos monorepo, this change adds a line to jest.config.js to re-map imports to `@react-native-mac/virtualized-lists`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please select one of the following
Summary:
It would be better for any eventual "monorepo" efforts for react-native desktop if react-native-macos did not directly modify JS files from react-native mobile. This change copies most JavaScript files that have been modified for react-native-macos to a new file with a *.macos.js platform extension. It also retains history following the approach described here. Basically:
Commit 1 (from main): Move files to preferred location
Commit 2 (from main): Move files to random location
Commit 3 (from Commit 1): Merge in Commit 2
Commit 4 (from Commit 3): Move files moved in Commit 2 back to original location
Files that I explicitly did not include in this PR were *.d.ts files (as these cannot be easily forked via platform extensions AFAIK), any native files in packages/react-native/Libraries, and any test files.
In order for history to be preserved, it is critical that the pull request is merged with "Rebase" (i.e., retain all commits).
Changelog:
[INTERNAL] [FIXED] - File rename trickery to duplicate file history and move forked macOS JS files to *.macos.js
Test Plan:
I made another commit and merged it into this commit, I confirmed that the merged commit only applies to the original upstream file (so merges from facebook/react-native should continue to work as before).
TODOs