Skip to content
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

**fix** findComponentDescriptors should look in codegenConfig/jsSrcsDir #2456

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mfazekas
Copy link

only and it should look into components starging with Native or ending with NativeComponent

Fixes: #2455

Summary:

findComponentDescriptors looks in all javascript files for regex codegenNativeComponent.... And android build uses that for autolinking.

Note that this is not 100% solution, as we still use regex parsing instead of parsing typescript and we also duplicate the logic from codegen. A better solution could be using codegen schema to parse for components.

Test Plan:

git clone https://github.com/mfazekas/rn-075-autolink
cd rn-075-autolink
git checkout autolink-wrong-files
cd ReproducerApp
node $RNCLI/packages/cli/build/bin.js config | grep componentDescriptors -A 3
          "componentDescriptors": [
            "RTNCenteredTextComponentDescriptor"
          ],

Works with .android platform extension

mv ../RTNCenteredText/js/RTNCenteredTextNativeComponent.ts ../RTNCenteredText/js/RTNCenteredTextNativeComponent.android.ts
node $RNCLI/packages/cli/build/bin.js config | grep componentDescriptors -A 3
          "componentDescriptors": [
            "RTNCenteredTextComponentDescriptor"
          ],

File matching Native*

echo 'codegenNativeComponent<NativeProps>("Foo")' > ../RTNCenteredText/js/NativeFoo.jsx
node $RNCLI/packages/cli/build/bin.js config | grep componentDescriptors -A 3
          "componentDescriptors": [
            "FooComponentDescriptor",
            "RTNCenteredTextComponentDescriptor"
          ],

File not matching Native* or NativeComponent

echo 'codegenNativeComponent<NativeProps>("Bar")' > ../RTNCenteredText/js/_Bar.jsx
node $RNCLI/packages/cli/build/bin.js config | grep componentDescriptors -A 3
          "componentDescriptors": [
            "FooComponentDescriptor",
            "RTNCenteredTextComponentDescriptor"
          ],

Checklist

@cortinico
Copy link
Member

only and it should look into components starging with Native or ending with NativeComponent

I believe that's not a correct fix. In theory users could device to don't use codegen (so you won't have codegenConfig/jsSrcsDir at all) but still have component descriptors that needs to be linked against

@mfazekas
Copy link
Author

I believe that's not a correct fix. In theory users could device to don't use codegen (so you won't have codegenConfig/jsSrcsDir at all) but still have component descriptors that needs to be linked against

Thanks for looking into it.

So the use case I've faced was that I've had a spec which I hasn't finished, so I've moved it away into temp/TODOComponent.ts. But auto linking was picking it up, which I've assumed must have been some stale cache from previous runs so it took me a while to figure out what's going on.

Another issue is an iOS only component - like src/specs/AppleOnlyNativceComponent.ios.ts will be picked up by auto link.

I'm not sure about the use case you've mentioned, where there is no codegen used, yet user want autolink to be generated. Based on javascript code spec. I guess one would need to create Specs.h or similar on its own. This change could definitely break such use cases.

https://github.com/react-native-community/cli/blob/main/docs/autolinking.md

So some of the possible steps:
1.) Improve only docs to describe what's going on
2.) Ignore files with .ios extension only
3.) only restrict the search to codegenConfig/jsSrcsDir if that's defined. So there is codegen config.

What do you think?

…jsSrcsDir only and it should look into components starging with Native or ending with NativeComponent
@cortinico
Copy link
Member

I'm not sure about the use case you've mentioned, where there is no codegen used, yet user want autolink to be generated. Based on javascript code spec. I guess one would need to create Specs.h or similar on its own. This change could definitely break such use cases.

You can always provide your own react-native.config.js and circumvent this logic alltoghether

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

autolink scans non used components
2 participants