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

fix: declarations of ContentSwitcher, Switch, Slider, SkeletonIcon #15382

Merged

Conversation

wkeese
Copy link
Contributor

@wkeese wkeese commented Dec 12, 2023

Addresses errors like:

TS2305: Module '"@carbon/react"' has no exported member 'ContentSwitcher'.

12 import { ContentSwitcher } from "@carbon/react";

This contains two types of fixes:

  1. Whenever the underlying component is written in Typescript, the index file needs to be named index.ts. If it's named index.js, the build won't even generate an index.d.ts file. (The index file can also be named index.tsx, but that doesn't make sense unless it contains React markup.)

  2. I updated createClassWrapper() so that Typescript knows the type of the returned functional component:

export function createClassWrapper<Props>(
  Component: ComponentClass<Props>
): FunctionComponent<Props> {
  ...
}

Note that the generated index.d.ts are a bit weird, for example

/// <reference types="react" />
declare const Slider: import("react").FunctionComponent<import("./Slider").SliderProps>;

If I add casting to the index.ts files themselves, ex:

const Slider = createClassWrapper(SliderComponent) as FC<SliderProps>;

That generates more canonical index.d.ts files:

import { FC } from "react";
import { SliderProps } from './Slider';
declare const Slider: FC<SliderProps>;

(So I can do that if you like.)

Of course, the createClassWrapper() calls should be temporary, as all remaining class components should be converted to functional components.

Refs #13550, #12557, #13574.

Copy link

netlify bot commented Dec 12, 2023

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 80761f2
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/6579019814cf9c000847c411
😎 Deploy Preview https://deploy-preview-15382--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@wkeese wkeese changed the title fix: declarations of ContentSwitcher, Slider, SkeletonIcon fix: declarations of ContentSwitcher, Switch, Slider, SkeletonIcon Dec 13, 2023
Addresses errors like:

TS2305: Module '"@carbon/react"' has no exported member 'ContentSwitcher'.
12 import { ContentSwitcher } from "@carbon/react";

This contains two types of fixes:

Whenever the underlying component is written in Typescript, the index file needs
to be named index.ts. If it's named index.js, the build won't even generate an
index.d.ts file. (The index file can also be named index.tsx, but that doesn't
make sense unless it contains React markup.)

I updated createClassWrapper() so that Typescript knows the type of the returned
functional component:

export function createClassWrapper<Props>(
  Component: ComponentClass<Props>
): FunctionComponent<Props> {
  ...
}

Note that the generated index.d.ts are a bit weird, for example

/// <reference types="react" />
declare const Slider:
   import("react").FunctionComponent<import("./Slider").SliderProps>;

If I add casting to the index.ts files themselves, ex:

const Slider = createClassWrapper(SliderComponent) as FC<SliderProps>;

That generates more canonical index.d.ts files:

import { FC } from "react";
import { SliderProps } from './Slider';
declare const Slider: FC<SliderProps>;

(So I can do that if you like.)

Of course, the createClassWrapper() calls should be temporary, as all remaining
class components should be converted to functional components.

Refs #13550, #12557, #13574.
@wkeese wkeese force-pushed the fix/typescript-for-class-components branch from a26cadc to 80761f2 Compare December 13, 2023 00:57
@wkeese wkeese marked this pull request as ready for review December 13, 2023 00:58
@wkeese wkeese requested a review from a team as a code owner December 13, 2023 00:58
Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

Thanks for this! I think the existing way is fine, canonical looks nicer but they're generated files and nobody's going to be actually reading or maintaining them anyway. I don't think there's any functional difference between the two?

@wkeese
Copy link
Contributor Author

wkeese commented Dec 13, 2023

Yeah I agree. I haven't finished converting our project to Carbon 11 yet but after this change, I don't get the error about
ContentSwitcher anymore.

@github-actions github-actions bot added this pull request to the merge queue Dec 18, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Dec 18, 2023
@tw15egan tw15egan added this pull request to the merge queue Dec 19, 2023
Merged via the queue into carbon-design-system:main with commit b2cc298 Dec 19, 2023
22 checks passed
@wkeese wkeese deleted the fix/typescript-for-class-components branch January 18, 2024 07:37
danoro96 pushed a commit to danoro96/carbon that referenced this pull request Jan 18, 2024
…sign-system#15382)

Addresses errors like:

TS2305: Module '"@carbon/react"' has no exported member 'ContentSwitcher'.
12 import { ContentSwitcher } from "@carbon/react";

This contains two types of fixes:

Whenever the underlying component is written in Typescript, the index file needs
to be named index.ts. If it's named index.js, the build won't even generate an
index.d.ts file. (The index file can also be named index.tsx, but that doesn't
make sense unless it contains React markup.)

I updated createClassWrapper() so that Typescript knows the type of the returned
functional component:

export function createClassWrapper<Props>(
  Component: ComponentClass<Props>
): FunctionComponent<Props> {
  ...
}

Note that the generated index.d.ts are a bit weird, for example

/// <reference types="react" />
declare const Slider:
   import("react").FunctionComponent<import("./Slider").SliderProps>;

If I add casting to the index.ts files themselves, ex:

const Slider = createClassWrapper(SliderComponent) as FC<SliderProps>;

That generates more canonical index.d.ts files:

import { FC } from "react";
import { SliderProps } from './Slider';
declare const Slider: FC<SliderProps>;

(So I can do that if you like.)

Of course, the createClassWrapper() calls should be temporary, as all remaining
class components should be converted to functional components.

Refs carbon-design-system#13550, carbon-design-system#12557, carbon-design-system#13574.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants