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

🐛 Bug - React Key type is wrong #1834

Open
denkristoffer opened this issue Jan 28, 2022 · 1 comment
Open

🐛 Bug - React Key type is wrong #1834

denkristoffer opened this issue Jan 28, 2022 · 1 comment
Assignees
Labels
bug Something isn't working as expected stale Used to mark when there was no activity for a set period of time

Comments

@denkristoffer
Copy link
Collaborator

denkristoffer commented Jan 28, 2022

Forma 36 bug report

Summary

The types returned for our components seem to override React's default key type, which allows null. Ours do not.

Details

I'm not sure how to share typescript examples that actually show type errors, so I will have to share a simple example the manual way:

import React from 'react';
import { Note } from '@contentful/f36-components';
import type { NoteProps } from '@contentful/f36-components';

export const MyNote = (props: NoteProps) => (
  <Note {...props}>
    {props.children}
  </Note>
);

Typescript will complain about this:

Types of property 'key' are incompatible.
Type 'Key | null | undefined' is not assignable to type 'Key | undefined'.
Type 'null' is not assignable to type 'Key | undefined'.

<Note {...props}>
~~~~

Digging into the NoteProps we can see that it takes key?: React.Key:

export const Note: React.ForwardRefExoticComponent<{
    color?: string;
    translate?: "no" | "yes";
    slot?: string;
    key?: React.Key;
...

React.Key is a type for string | number. If we dig into @types/react we find this:

    /**
     * @internal You shouldn't need to use this type since you never see these attributes
     * inside your component or have to validate them.
     */
    interface Attributes {
        key?: Key | null | undefined;
    }
...

So as you can see, our types are removing the ability to set key to null. I don't think we should be redefining this at all.

@github-actions
Copy link

github-actions bot commented Aug 9, 2022

Marking issue as stale since there was no acitivty for 30 days

@github-actions github-actions bot added the stale Used to mark when there was no activity for a set period of time label Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected stale Used to mark when there was no activity for a set period of time
Projects
None yet
Development

No branches or pull requests

6 participants