Fixed wrong usage of require when resolving supportsCodegenConfig in react-native.config.js #657
Conversation
When resolving the version inside `react-native-config` which is used by the autolinking gradle tasks, the wrong use of require was used. This fix changes from using `require.main.require` -> `require`. The difference is that require.main.require resolves from the entry point (main script), while require resolves from the location of the current file. Closes callstack#652
|
Just to add a bit of context: the config output is used to generate but prior to this change, the If I may, I'd even suggest the maintainers to remove the |
|
@vonovak agree! We support 3 latest versions, so version 9.x is not longer supported 👍 |
|
All good - will you be merging and making a release, @szymonrybczak? Then we can update our Expo versions to the latest so this one is fixed :) Thanks in advance! |
|
cc @BartoszKlonowski do you mind taking a look? 🤔 |
We've had some issue with monorepos when resolving the codegen config on Android in other libraries, and found that due to versions no longer in use we could now remove the version test in `react-native-config.js` and always return the component descriptor etc. (The monorepo issue was with using `require.main.require` which will resolve to the calling script and not the module) For reference, here is the same PR in @react-native-community/slider: callstack/react-native-slider#657
There was a problem hiding this comment.
I've got nothing here to complain about, good job, everyone 👍
Thank you, @chrfalch, for handling this!
(Sorry for the delay, but for such cases I'm available on Discord and Slack, wherever possible for one. Still, you managed this just great on your own, folks 💪)
|
Oh, one more thing: I don't know if I'll be able to 🚢 during the weekend, so if not, then please expect this released on Monday. |
All good - Monday will be more than good enough - thanks :) |
* Removed a check that is no longer needed We've had some issue with monorepos when resolving the codegen config on Android in other libraries, and found that due to versions no longer in use we could now remove the version test in `react-native-config.js` and always return the component descriptor etc. (The monorepo issue was with using `require.main.require` which will resolve to the calling script and not the module) For reference, here is the same PR in @react-native-community/slider: callstack/react-native-slider#657 * cr: Reverted and simplified change after code review Removed previous change and made the `supportsCodegenConfig` variable true initially * revert: reverted commit - removed last line.
Summary:
When using the library in monorepo solutions or other non-standard types of projects the Slider will end up with a height of 0 and not work. (See #652)
Problem/fix:
When resolving the version inside
react-native-configwhich is used by the autolinking gradle tasks, the wrong use of require was used.This fix changes from using
require.main.require->require.The difference is that require.main.require resolves from the entry point (main script), while require resolves from the location of the current file.
Fixes (partially) #652
Test Plan:
See #652 for reproduction and tests. We've tested in in our BareExpo project, in a regular project (where it also previously did work) and in a project using react-native-test-app (https://github.com/vonovak/react-native-slider/tree/fix/new-arch-issues/new-example)