refactor(react): rename createReactComponent to createComponentImplementation#1024
refactor(react): rename createReactComponent to createComponentImplementation#1024ppazosp wants to merge 1 commit intogoogle:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request renames createReactComponent to createComponentImplementation across the React renderer's adapter, components, and tests. Feedback identifies a violation of the repository's style guide regarding the pre-review checklist and suggests that this breaking change be documented in a new CHANGELOG entry for better visibility. Furthermore, it is recommended to rename createBinderlessComponent to ensure naming consistency across the API.
| ## 0.8.1 | ||
|
|
||
| - Use the `InferredComponentApiSchemaType` from `web_core` in `createReactComponent`. | ||
| - Use the `InferredComponentApiSchemaType` from `web_core` in `createComponentImplementation`. |
There was a problem hiding this comment.
The 'I have added updates to the [CHANGELOG]' checkbox in the PR description is unchecked, which is confusing as this file is modified. The repository style guide requires the pre-review checklist to be fully completed.
More importantly, renaming an exported function is a breaking change and should be explicitly documented. The current change modifies an existing entry, which may not give enough visibility to this important update.
Please add a new entry, perhaps under an ## Unreleased section, to clearly communicate this breaking change. For example:
## Unreleased
- **BREAKING CHANGE**: Renamed `createReactComponent` to `createComponentImplementation`.References
- PR descriptions should include the Pre-Review Checklist from the PR template, with all of the steps completed. (link)
There was a problem hiding this comment.
Done — added an Unreleased section with both breaking changes documented. Also renamed createBinderlessComponent to createBinderlessComponentImplementation for consistency as suggested in the other comment.
| * Creates a React component implementation using the deep generic binder. | ||
| */ | ||
| export function createReactComponent<Api extends ComponentApi>( | ||
| export function createComponentImplementation<Api extends ComponentApi>( |
There was a problem hiding this comment.
For consistency with this rename, consider also renaming createBinderlessComponent (at line 107) to createBinderlessComponentImplementation. Both functions return a ReactComponentImplementation, so it would make their purpose clearer and more parallel if their names followed a similar pattern.
There was a problem hiding this comment.
Good catch — done. Renamed createBinderlessComponent to createBinderlessComponentImplementation in the same commit.
ppazosp
force-pushed
the
refactor/rename-createReactComponent
branch
from
0b8afa6 to
4000538
Compare
…entation Also renames createBinderlessComponent to createBinderlessComponentImplementation for consistency. Closes google#974
ppazosp
force-pushed
the
refactor/rename-createReactComponent
branch
from
4000538 to
27e2642
Compare
1 participant
Description
Renames
createReactComponenttocreateComponentImplementationacross the React v0.9 renderer.As noted in #974, React developers will only be using this function within the React renderer context, so including "React" in the name is redundant. The new name better describes what the function does: it creates a component implementation from a
ComponentApidefinition.Changes:
adapter.tsx(definition + export)adapter.test.tsxCHANGELOG.mdreferenceAll 397 existing tests pass.
Closes #974
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.