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

feat(legacy view template): extract view to a separate file #375

Closed
wants to merge 5 commits into from

Conversation

atlj
Copy link
Collaborator

@atlj atlj commented Mar 21, 2023

Summary

Closes #370

Inspecting the codebase I've spotted that all of the view templates have the View initialized on a different file except the legacy view template. Having the view initialized in the index.tsx reduces the DX as anytime you make changes to the 'index.tsxfile, you will getInvariant Violation: Tried to register two views with the same name` error. This PR addresses these issues.

Test plan

  1. Run create-react-native-library and select these options:
➜ ./create-react-native-library test
✔ What is the name of the npm package? … react-native-test
✔ What is the description for the package? … test
✔ What is the name of package author? … Burak Güner
✔ What is the email address for the package author? … [email protected]
✔ What is the URL for the package author? … https://github.com/atlj
✔ What is the URL for the repository? … https://github.com/atlj/react-native-test
✔ What type of library do you want to develop? › Native view
✔ Which languages do you want to use? › Kotlin & Swift
  1. Go inside src/
  2. Make sure <Your Project Name>ViewNativeComponent.ts exists with proper code
  3. Make sure the view is imported and exposed in index.tsx
  4. Make sure the app is able to display the view
  5. Make sure when you make changes to index.tsx, the Invariant Violation: Tried to register two views with the same name error doesn't appear.

@atlj atlj requested a review from satya164 March 21, 2023 15:27
Copy link
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also do it for native modules and then merge

@satya164 satya164 force-pushed the main branch 3 times, most recently from 7b11914 to 6facf3b Compare July 4, 2023 17:35
@@ -0,0 +1,25 @@
import {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's name this file as <%- project.name -%>View.tsx to match the name of the component it exports


const ComponentName = '<%- project.name -%>View';

export default UIManager.getViewManagerConfig(ComponentName) != null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally good to avoid default exports

Suggested change
export default UIManager.getViewManagerConfig(ComponentName) != null
export const <%- project.name -%>View = UIManager.getViewManagerConfig(ComponentName) != null

@atlj atlj requested a review from satya164 October 27, 2023 18:29
@atlj
Copy link
Collaborator Author

atlj commented Oct 27, 2023

You can find the diff here

@satya164
Copy link
Member

satya164 commented Feb 2, 2024

Let's move the index file to common directory and change each type (legacy, new, mixed) to have this format for the view and module.

@satya164
Copy link
Member

Burak said we don't need this anymore

@satya164 satya164 closed this Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invariant Violation: Tried to register two views with the same name
2 participants