-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Components: Rename PolymorphicComponent*
types to WordPressComponent*
#34330
Components: Rename PolymorphicComponent*
types to WordPressComponent*
#34330
Conversation
Size Change: 0 B Total Size: 1.04 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Thanks, @ciampo!
I noticed that in some places we're using import type
and in others we're just importing the types directly. Are we following any convention in that regard?
From what I can gather, until recently our preferred option in TypeScript files was to keep the type imports separate from importing runtime code. I believe this technique was used in an attempt to keep the bundle size as small as possible, e.g. import { myFunction } from './file';
import type { MyType } from './file'; Recently, though, it was discussed and agreed that optimisation isn't really necessary, and so we started switching to merging those import statements into one, e.g. import { myFunction, MyType } from './file'; Still, it is possible to still encounter an import type { MyType, MyOtherType } from './file'; Following the rationale that I just explained, I had a look at the code in this PR and merged import declarations where possible in |
6508cd9
to
faa2953
Compare
All CI tasks seemed to be failing after the last commit — I've tried to rebase on top of Update: the issues seem to persist Update: all good after last rebase! Going to merge |
faa2953
to
c53dec6
Compare
c53dec6
to
9747a6a
Compare
Description
Renames the following types:
PolymorphicComponent
WordPressComponent
PolymorphicComponentProps
WordPressComponentProps
PolymorphicComponentFromProps
WordPressComponentFromProps
This PR also renames the file containing the type definitions from
polymoprhic-component.ts
towordpress-component.ts
, and adds a CHANGELOG entry.How has this been tested?
The project builds correctly, tests pass, smoke tested in Storybook.
Screenshots
N/A
Types of changes
Refactor (type renaming)
Checklist:
*.native.js
files for terms that need renaming or removal).Closes #34291