-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Breaking: Remove BaseViewManagerInterface #46809
Conversation
This pull request was exported from Phabricator. Differential Revision: D63819044 |
Summary: BaseViewManagerInterface isn't adding much value right now. It was added in D16984121 to allow codegen generated ViewManager delegates to apply to view managers which derive from ViewMangager instead of BaseViewManager (if they did some cleverness, to make VM delegate apply to a no-op class, still implementing all of BaseViewManager's methods). All of the cases where that was used have since been moved to `SimpleViewManager`, and `BaseViewManagerAdapter` (needed to wire this together) doesn't exist anymore, so it's not possible to take any advantage of this interface existing. We should remove it, since its existence is a source of error (e.g. it was missing setters for `accessibilityValue` or those related to pointer events), and is more generally confusing for anyone adding to `BaseViewManager` in the future. This is a breaking change, because there are some libraries which vendor a copy of generated ViewManagerDelegate when building against legacy arch to be able to share code normally generated at build time. That means these will need to be updated to maintain compatibility with RN versions of 0.77+ with new arch disabled. This will not effect compatibility of these libraries against the default new arch, and the updated delegate is still compatible with older RN version. ``` sourceSets.main { java { if (!isNewArchitectureEnabled()) { srcDirs += [ "src/paper/java", ] } } } ``` 1. `rnmapbox/maps` 2. `react-native-gesture-handler` 3. `react-native-screens` 4. `react-native-svg` 5. `react-native-safe-area-context` 6. `react-native-pdf` Changelog: [Android][Breaking] - Remove BaseViewManagerInterface Differential Revision: D63819044
0104b97
to
7a31dad
Compare
This pull request was exported from Phabricator. Differential Revision: D63819044 |
Summary: BaseViewManagerInterface isn't adding much value right now. It was added in D16984121 to allow codegen generated ViewManager delegates to apply to view managers which derive from ViewMangager instead of BaseViewManager (if they did some cleverness, to make VM delegate apply to a no-op class, still implementing all of BaseViewManager's methods). All of the cases where that was used have since been moved to `SimpleViewManager`, and `BaseViewManagerAdapter` (needed to wire this together) doesn't exist anymore, so it's not possible to take any advantage of this interface existing. We should remove it, since its existence is a source of error (e.g. it was missing setters for `accessibilityValue` or those related to pointer events), and is more generally confusing for anyone adding to `BaseViewManager` in the future. This is a breaking change, because there are some libraries which vendor a copy of generated ViewManagerDelegate when building against legacy arch to be able to share code normally generated at build time. That means these will need to be updated to maintain compatibility with RN versions of 0.77+ with new arch disabled. This will not effect compatibility of these libraries against the default new arch, and the updated delegate is still compatible with older RN version. ``` sourceSets.main { java { if (!isNewArchitectureEnabled()) { srcDirs += [ "src/paper/java", ] } } } ``` 1. `rnmapbox/maps` 2. `react-native-gesture-handler` 3. `react-native-screens` 4. `react-native-svg` 5. `react-native-safe-area-context` 6. `react-native-pdf` Changelog: [Android][Breaking] - Remove BaseViewManagerInterface Differential Revision: D63819044
7a31dad
to
2f2f9da
Compare
This pull request was exported from Phabricator. Differential Revision: D63819044 |
2f2f9da
to
70e03d6
Compare
Summary: BaseViewManagerInterface isn't adding much value right now. It was added in D16984121 to allow codegen generated ViewManager delegates to apply to view managers which derive from ViewMangager instead of BaseViewManager (if they did some cleverness, to make VM delegate apply to a no-op class, still implementing all of BaseViewManager's methods). All of the cases where that was used have since been moved to `SimpleViewManager`, and `BaseViewManagerAdapter` (needed to wire this together) doesn't exist anymore, so it's not possible to take any advantage of this interface existing. We should remove it, since its existence is a source of error (e.g. it was missing setters for `accessibilityValue` or those related to pointer events), and is more generally confusing for anyone adding to `BaseViewManager` in the future. This is a breaking change, because there are some libraries which vendor a copy of generated ViewManagerDelegate when building against legacy arch to be able to share code normally generated at build time. That means these will need to be updated to maintain compatibility with RN versions of 0.77+ with new arch disabled. This will not effect compatibility of these libraries against the default new arch, and the updated delegate is still compatible with older RN version. ``` sourceSets.main { java { if (!isNewArchitectureEnabled()) { srcDirs += [ "src/paper/java", ] } } } ``` 1. `react-native-picker/picker` 2. `rnmapbox/maps` 3. `react-native-gesture-handler` 4. `react-native-screens` 5. `react-native-svg` 6. `react-native-safe-area-context` 7. `react-native-pdf` Changelog: [Android][Breaking] - Remove BaseViewManagerInterface Reviewed By: cortinico Differential Revision: D63819044
This pull request was exported from Phabricator. Differential Revision: D63819044 |
Summary: BaseViewManagerInterface isn't adding much value right now. It was added in D16984121 to allow codegen generated ViewManager delegates to apply to view managers which derive from ViewMangager instead of BaseViewManager (if they did some cleverness, to make VM delegate apply to a no-op class, still implementing all of BaseViewManager's methods). All of the cases where that was used have since been moved to `SimpleViewManager`, and `BaseViewManagerAdapter` (needed to wire this together) doesn't exist anymore, so it's not possible to take any advantage of this interface existing. We should remove it, since its existence is a source of error (e.g. it was missing setters for `accessibilityValue` or those related to pointer events), and is more generally confusing for anyone adding to `BaseViewManager` in the future. This is a breaking change, because there are some libraries which vendor a copy of generated ViewManagerDelegate when building against legacy arch to be able to share code normally generated at build time. That means these will need to be updated to maintain compatibility with RN versions of 0.77+ with new arch disabled. This will not effect compatibility of these libraries against the default new arch, and the updated delegate is still compatible with older RN version. ``` sourceSets.main { java { if (!isNewArchitectureEnabled()) { srcDirs += [ "src/paper/java", ] } } } ``` 1. `react-native-picker/picker` 2. `rnmapbox/maps` 3. `react-native-gesture-handler` 4. `react-native-screens` 5. `react-native-svg` 6. `react-native-safe-area-context` 7. `react-native-pdf` Changelog: [Android][Breaking] - Remove BaseViewManagerInterface Reviewed By: cortinico Differential Revision: D63819044
70e03d6
to
57bfd89
Compare
This pull request was exported from Phabricator. Differential Revision: D63819044 |
Summary: BaseViewManagerInterface isn't adding much value right now. It was added in D16984121 to allow codegen generated ViewManager delegates to apply to view managers which derive from ViewMangager instead of BaseViewManager (if they did some cleverness, to make VM delegate apply to a no-op class, still implementing all of BaseViewManager's methods). All of the cases where that was used have since been moved to `SimpleViewManager`, and `BaseViewManagerAdapter` (needed to wire this together) doesn't exist anymore, so it's not possible to take any advantage of this interface existing. We should remove it, since its existence is a source of error (e.g. it was missing setters for `accessibilityValue` or those related to pointer events), and is more generally confusing for anyone adding to `BaseViewManager` in the future. This is a breaking change, because there are some libraries which vendor a copy of generated ViewManagerDelegate when building against legacy arch to be able to share code normally generated at build time. That means these will need to be updated to maintain compatibility with RN versions of 0.77+ with new arch disabled. This will not effect compatibility of these libraries against the default new arch, and the updated delegate is still compatible with older RN version. ``` sourceSets.main { java { if (!isNewArchitectureEnabled()) { srcDirs += [ "src/paper/java", ] } } } ``` 1. `react-native-picker/picker` 2. `rnmapbox/maps` 3. `react-native-gesture-handler` 4. `react-native-screens` 5. `react-native-svg` 6. `react-native-safe-area-context` 7. `react-native-pdf` Changelog: [Android][Breaking] - Remove BaseViewManagerInterface Reviewed By: cortinico Differential Revision: D63819044
This pull request has been merged in 7fb3d83. |
This pull request was successfully merged by @NickGerleman in 7fb3d83 When will my fix make it into a release? | How to file a pick request? |
The change is required because BaseViewManagerInterface has been removed in facebook/react-native#46809 and we need this to keep the lib working on old architecture with react-native 0.77+. This change, according to the PR is supposed to be backward compatible (older versions of react-native) should work with these new codegen specs.
Summary:
BaseViewManagerInterface isn't adding much value right now. It was added in D16984121 to allow codegen generated ViewManager delegates to apply to view managers which derive from ViewMangager instead of BaseViewManager (if they did some cleverness, to make VM delegate apply to a no-op class, still implementing all of BaseViewManager's methods).
All of the cases where that was used have since been moved to
SimpleViewManager
, andBaseViewManagerAdapter
(needed to wite this together) doesn't exist anymore, so it's not possible to take any advantage of this interface existing. We should remove it, since its existence is a sources of error (e.g. it was missing setters foraccessibilityValue
or those related to pointer events), and is more generally confusing for anyone adding toBaseViewManager
in the future.This is a breaking change, because there are some libraries which vendor a copy of generated ViewManagerDelegate when building against legacy arch to be able to share code normally generated at build time. That means these will need to be updated to maintain compatibility with RN versions of 0.77+ with new arch disabled. This will not effect compatibility of these libraries against the default new arch, and the updated delegate is still compatible with older RN version.
The following libraries follow this pattern:
rnmapbox/maps
react-native-gesture-handler
react-native-screens
react-native-svg
react-native-safe-area-context
react-native-pdf
Changelog:
[Android][Breaking] - Remove BaseViewManagerInterface
Differential Revision: D63819044