Fix content/en/guide/v10/typescript.md #718
Conversation
|
I think it's not only a type for children that's added to |
Does the original description worth it? |
| @@ -7,8 +7,6 @@ description: "Preact has built-in TypeScript support. Learn how to make use of i | |||
|
|
|||
| Preact ships TypeScript type definitions, which are used by the library itself! | |||
|
|
|||
| When you use Preact in a TypeScript-aware editor (like VSCode), you can benefit from the added type information while writing regular JavaScript. If you want to add type information to your own applications, you can use [JSDoc annotations](https://fettblog.eu/typescript-jsdoc-superpowers/), or write TypeScript and transpile to regular JavaScript. This section will focus on the latter. | |||
38elements
Nov 26, 2020
Author
Contributor
This is a description of the VSCode features and methods of adding types.
I think it is unnecessary in Preact document.
Therefore, I removed this line.
|
You're right, this might be misleading in the original version as well. What about:
This should be correct. |
| @@ -92,7 +90,7 @@ function Greeting({ name = "User" }: GreetingProps) { | |||
| } | |||
| ``` | |||
|
|
|||
| Preact also ships a `FunctionComponent` type to annotate anonymous functions. `FunctionComponent` also adds a type for `children`: | |||
| Preact ships a `FunctionComponent` type to annotate anonymous functions. `FunctionComponent` adds a type for `children` to `props`: | |||
38elements
Nov 27, 2020
Author
Contributor
This describes what a type for children is added to.
https://github.com/preactjs/preact/blob/f955cfcceb50d504faa7c05a95065512cf900e57/src/index.d.ts#L72-L90
| @@ -170,7 +170,7 @@ class Expandable extends Component<ExpandableProps, ExpandableState> { | |||
|
|
|||
| ## Typing events | |||
|
|
|||
| Preact emits regular DOM events. As long as your TypeScript project includes the `dom` library (set it in `tsconfig.json`), you have access to all event types that are available in your current configuration. | |||
38elements
Dec 4, 2020
Author
Contributor
In Preact core,
onChangeis the standard [DOM change event]
There is "standard DOM event" in other page.
Therefore, I change to "standard DOM event".
Element emits event.
| @@ -170,7 +170,7 @@ class Expandable extends Component<ExpandableProps, ExpandableState> { | |||
|
|
|||
| ## Typing events | |||
|
|
|||
| Preact handles standard DOM events. As long as your TypeScript project includes the `dom` library (set it in `tsconfig.json`), you have access to all event types that are available in your current configuration. | |||
38elements
Dec 5, 2020
Author
Contributor
lib includes DOM.
standard DOM events types is more explicit.
| @@ -187,7 +187,7 @@ export class Button extends Component { | |||
| } | |||
| ``` | |||
|
|
|||
| You can restrict event handlers by adding a type annotation for `this` to the function signature as the first argument. This argument will be erased after transpilation. | |||
38elements
Dec 6, 2020
Author
Contributor
This content is an explanation about this parameters.
Therefore, This is not appropriate in Preact document.
|
|
||
| ```tsx | ||
| export class Button extends Component { | ||
| handleClick(event: MouseEvent) { | ||
| event.preventDefault(); | ||
| if (event.target instanceof HTMLElement) { | ||
| alert(event.target.tagName); // Alerts BUTTON |
| render() { | ||
| return ( | ||
| <div class="expandable"> | ||
| <h2> | ||
| // `this.props.title` is string type by ExpandableProps. |
developit
Dec 7, 2020
Member
I need to check if our syntax highlighter can handle line comments in JSX
|
|
||
| ```tsx | ||
| export class Button extends Component { | ||
| handleClick(event: MouseEvent) { | ||
| event.preventDefault(); | ||
| if (event.target instanceof HTMLElement) { | ||
| alert(event.target.tagName); // Alerts BUTTON |
| @@ -7,8 +7,6 @@ description: "Preact has built-in TypeScript support. Learn how to make use of i | |||
|
|
|||
| Preact ships TypeScript type definitions, which are used by the library itself! | |||
|
|
|||
| When you use Preact in a TypeScript-aware editor (like VSCode), you can benefit from the added type information while writing regular JavaScript. If you want to add type information to your own applications, you can use [JSDoc annotations](https://fettblog.eu/typescript-jsdoc-superpowers/), or write TypeScript and transpile to regular JavaScript. This section will focus on the latter. | |||
1d4f948
to
881fe90
| if (event.target instanceof HTMLElement) { | ||
| console.log(event.target.localName); // "button" | ||
| } | ||
| console.log(this.tagName); // "BUTTON" | ||
| } |
38elements
Dec 8, 2020
Author
Contributor
I think this example should use this.
Since this is HTMLButtonElement, instanceof is unnecessary.
Another example uses tagName.
ddprrt
Dec 8, 2020
Contributor
I agree, this is much more concise and showcases this binding much better
| ref = createRef<HTMLAnchorElement>(); | ||
| componentDidMount() { | ||
| // current is of type HTMLAnchorElement |
| } | ||
| } | ||
| ``` | ||
|
|
||
| This helps a lot if you want to make sure that the elements you `ref` to are input elements that can be e.g. focussed. | ||
|
|
||
| ## Typing context | ||
|
|
||
| `createContext` tries to infer as much as possible from the intial values you pass to: |
|
|
||
| ### Function components |
38elements
Dec 11, 2020
Author
Contributor
They are "Functional Components" and "Class Components" in other page.
https://github.com/preactjs/preact-www/blob/922098858eba2769900cc909b18b7b27e5487a49/content/en/guide/v10/components.md#functional-components
| @@ -92,7 +93,7 @@ function Greeting({ name = "User" }: GreetingProps) { | |||
| } | |||
| ``` | |||
|
|
|||
| Preact also ships a `FunctionComponent` type to annotate anonymous functions. `FunctionComponent` also adds a type for `children`: | |||
38elements
Dec 12, 2020
Author
Contributor
FunctionComponent type corresponds all functional components, not only an anonymous function.
This sentence is confusing.
|
|
||
| There are different ways to type components in Preact. Class components have generic type variables to ensure type safety. TypeScript sees a function as functional component as long as it returns JSX. There are multiple solutions to define props for functional components. |
38elements
Dec 12, 2020
Author
Contributor
TypeScript sees a function as functional component as long as it returns JSX.
Unless a function is matched FunctionComponent or FunctionalComponent, it is not considered a functional components.
This is incorrect.
38elements
Dec 12, 2020
Author
Contributor
Since there is the same content at below, I think this line is unnecessary.
This describes what a type for `children` is added to. https://github.com/preactjs/preact/blob/f955cfcceb50d504faa7c05a95065512cf900e57/src/index.d.ts#L72-L90
| You can set default props by setting a default value in the function signature. | ||
|
|
||
| ```tsx | ||
| type GreetingProps = { | ||
| name?: string; // name is optional! | ||
| } | ||
| function Greeting({ name = "User" }: GreetingProps) { | ||
| // name is at least "User" | ||
| return <div>Hello {name}!</div> | ||
| } | ||
| ``` |
38elements
Dec 12, 2020
•
Author
Contributor
This is explanation for Default parameters.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Default_parameters
Therefore, this is not appropriate in TypeScript page of Preact document.
marvinhagemeister
Dec 26, 2020
Member
I think the section was great and we should keep it. People switching from prop types over to TS will naturally ask these questions as most linters treat prop-types and default props as the same. We should cater to those users too
| @@ -81,7 +81,7 @@ type Props = { | |||
| age: number; | |||
| }; | |||
| function MyComponent({ name, age }: Props) { | |||
38elements
Dec 12, 2020
•
Author
Contributor
Most React libraries use React.FC.
Using FunctionComponent method is explicit.
Therefore, this example is unnecessary.
| <div hidden={this.state.toggled}>{this.props.children}</div> | ||
| </div> | ||
| ); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| Class components include children by default, typed as `ComponentChildren`. | ||
|
|
||
| ## Typing events |
38elements
Dec 13, 2020
Author
Contributor
I think it is necessary to describe about TargetedEvent, since TargetedEvent does not exist in React.
|
|
||
| The `createRef` function is also generic, and lets you bind references to element types. In this example, we ensure that the reference can only be bound to `HTMLAnchorElement`. Using `ref` with any other element lets TypeScript thrown an error: |
38elements
Dec 15, 2020
Author
Contributor
Using
refwith any other element lets TypeScript thrown an error.
This is explanation about TypeScript TypeError.
I think it is unnecessary in Preact document.
Therefore, I removed this sentence.
| render() { | ||
| return <div ref={this.ref}>Foo</div>; | ||
| // ~~~ | ||
| // 💥 Error! Ref only can be used for HTMLAnchorElement |
| @@ -173,18 +232,17 @@ class Expandable extends Component<ExpandableProps, ExpandableState> { | |||
| } | |||
| ``` | |||
|
|
|||
| Class components include children by default, typed as `ComponentChildren`. | |||
38elements
Dec 18, 2020
Author
Contributor
children? is default, not children.
type RenderableProps<P, RefType = any> = P & Readonly<Attributes & { children?: ComponentChildren; ref?: Ref<RefType> }>;
I sent this pull request because I thought it was worth it. |
| @@ -100,7 +100,7 @@ const MyComponent: FunctionComponent<Props> = function ({ name, age }) { | |||
| type RenderableProps<P, RefType = any> = P & Readonly<Attributes & { children?: ComponentChildren; ref?: Ref<RefType> }>; | |||
| ``` | |||
|
|
|||
| When `children` is required, it need to be specified: | |||
| Since `children` in `RenderableProps` type is `children?: ComponentChildren`, `children` need to be specified when `children` is required: | |||
| It also requires you to pass in all the properties you defined in the initial value: | ||
|
|
||
| ```tsx | ||
| function App() { | ||
| // This one errors 💥 as we haven't defined theme | ||
| return ( | ||
| <AppContext.Provider | ||
| value={{ | ||
| // ~~~~~ | ||
| // 💥 Error: theme not defined | ||
| lang: "de", | ||
| authenticated: true | ||
| }} | ||
| > | ||
| {} | ||
| <ComponentThatUsesAppContext /> | ||
| </AppContext.Provider> | ||
| ); | ||
| } | ||
| ``` |
38elements
Dec 20, 2020
•
Author
Contributor
This is mere explanation about TypeScript TypeError.
I think it is unnecessary in Preact document.
Therefore, I removed this sentence.
| If you don't want to specify all properties, you can either merge default values with overrides: | ||
|
|
||
| ```tsx | ||
| const AppContext = createContext(appContextDefault); | ||
| function App() { | ||
| return ( | ||
| <AppContext.Provider | ||
| value={{ | ||
| lang: "de", | ||
| ...appContextDefault | ||
| }} | ||
| > | ||
| <ComponentThatUsesAppContext /> | ||
| </AppContext.Provider> | ||
| ); | ||
| } | ||
| ``` |
38elements
Dec 20, 2020
Author
Contributor
This is mere explanation about how to pass values to props using destructuring assignment.
I think it is not suitable in this page.
Therefore, I removed this sentence.
| Or you work without default values and use bind the generic type variable to bind context to a certain type: | ||
|
|
||
| ```tsx | ||
| type AppContextValues = { | ||
| authenticated: boolean; | ||
| lang: string; | ||
| theme: string; | ||
| } | ||
| const AppContext = createContext<Partial<AppContextValues>>({}); | ||
| function App() { | ||
| return ( | ||
| <AppContext.Provider | ||
| value={{ | ||
| lang: "de" | ||
| }} | ||
| > | ||
| <ComponentThatUsesAppContext /> | ||
| </AppContext.Provider> | ||
| ); | ||
| ``` | ||
|
|
||
| All values become optional, so you have to do null checks when using them. |
38elements
Dec 20, 2020
•
Author
Contributor
This is mere explanation about Partial type.
I think it is unnecessary in Preact document.
Therefore, I removed this sentence.
| @@ -350,7 +358,7 @@ const Counter = ({ initial = 0 }) => { | |||
|
|
|||
| `useEffect` does extra checks so you only return cleanup functions. | |||
38elements
Dec 21, 2020
Author
Contributor
It seems this line is mere explanation about useEffect. Since Hooks page is suitable, this line is unnecessary.
| // ✅ if you return something from the effect callback | ||
| // it HAS to be a function without arguments |
38elements
Dec 21, 2020
•
Author
Contributor
It seems this lines is mere explanation about useEffect and TypeError.
No one passes an argument since this function does not accept arguments.
Therefore, this lines is unnecessary.
|
This PR has gotten a bit out of hand and is impossible to merge. Most of the changes here are made under the assumption that there should be a strict divide between what should be in the TS docs, on MDN or in ours. The line is much more blurry in reality and I vastly prefer the previous approach of including some tips in our docs, even if they can be found in detail in others scattered over the internet. I know this is just a draft, but we should strife to make smaller PRs. When they are more about corrections they become easier to review too |
|
@marvinhagemeister
I think this is a looking down way of saying it: "I think it's not only a type for children that's added to props but also the property children in general. Worth mentioning?" I can accept the following comments: "I think it's not only a type for children that's added to props but also the property children in general. Therefore, I think this change is unnecessary." I have already explained. And I sent it because I think it is worth it. I do not think it is a necessary question. I think you have not been never commented like this in Preact community. |
| // Types for props | ||
| // in the case `children` is required. |
| type ExpandableProps = { | ||
| title: string; | ||
| children: ComponentChildren; |

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.


This pull request is for reviewing #716.
Japanese version is added this change to.
I think #703 includes many repeating, incorrect, unnecessary things.