Comments (19)
In the meanwhile I found that the React team decided to deprecate defaultProps
for function component. 😱
In the next future we might consider getting rid of them and switch to JS default arguments even though this would mess up the code quite a bit.
from react-minimal-pie-chart.
Added an entry to docs' todo.
Please feel free to reopen in case the bug doesn't come from WebStorm.
from react-minimal-pie-chart.
Hi @mkarajohn,
thanks for reporting!
This could be related to the way we join props and defaultProps types. Let's find out.
I'm quite concerned about the fact that tests haven't caught the issue.
from react-minimal-pie-chart.
Hi,
This could be related to the way we join props and defaultProps types. Let's find out.
Yes, it seems to me as well, this is the cause.
I think the intersection in this line makes the defaultProps
required
You can see this TS sandbox example, for the same effect
from react-minimal-pie-chart.
I thought about it for a moment.
From the library's perspective PropsWithDefaults
describes the props
object (with default props) expected by PieChart
function component.
Default props are therefore marked as required because they are expected to be provided or defaulted by React. In my usual TS+React routine I can't get any typing error.
And PropsWithDefaults
is what you get when you retrieve components' props with:
React.ComponentProps<typeof PieChart>
Since pure Props
types alone are not exported, there's currently no way to retrieve the type definition of the props the user is expected to provide. Which is probably a quite common issue with React's defaultProps in TS.
I'm totally open to find a better typing setup.
BTW, can I see a repro of your typing error? Maybe you're using the library in a way I haven't taken into account.
Cheers! 🙌
from react-minimal-pie-chart.
My setup is WebStorm 2019.3.4 on Mac and Windows 10. I get the warning regardless of OS.
I am using the component like this
<PieChart
key={'proper'}
animate
animationDuration={500}
animationEasing="ease-out"
background={'#bfbfbf'}
css={chartStyles}
lineWidth={60}
data={chartData}
/>
I cannot disclose any more code than that.
Default props are therefore marked as required because they are expected to be provided or defaulted by React.
I don't think default props should be marked as required. On that regard, my IDE is right for throwing those warnings at me.
Think about it. Default props are optional props, in the end. Whether or not your component handles them with some default value is an implementation detail. To the user they should not be any different from any other optional props, during development time. One can refer to the docs to tell what props are defaulted with what values.
If I alter the definition in the Chart.d.ts
file to export declare function PieChart(props: Props): JSX.Element;
the Typescript gods are appeased.
Cheers!
from react-minimal-pie-chart.
Hi @mkarajohn,
crazy idea: could the issue be related to WebStorm?! I took a look to it's bug tracker and found a few reports that seem to be similar to what you described.
from react-minimal-pie-chart.
Not a crazy idea at all :D. It may be one of these issues, However i am not sure which one could it be. The 1st one in that list (which is very similar to this case) is closed as a duplicate of this which is supposed to be fixed. I don't know, honestly, proceed with this issue however you feel like.
from react-minimal-pie-chart.
In the meanwhile I found that the React team decided to deprecate defaultProps for function component. 😱
Yep, that's true. For the better, if you ask me
from react-minimal-pie-chart.
Hi, it's this time of the year again 😄
Well, I upgraded to TS 4.1.3 from 3.9.7 and I got this exact same error again. I actually had completely forgotten about this issue, until now, when I searched for it and got here again 😅
This time I am on Linux.
A quick search at JetBrain's issue tracker brought nothing
Still the only fix for me is to do export declare function PieChart(props: Props): JSX.Element;
// global.d.ts
declare module 'react-minimal-pie-chart' {
declare type Props = {
animate?: boolean;
animationDuration?: number;
animationEasing?: string;
background?: string;
center?: [number, number];
children?: ReactNode;
className?: string;
data: Data;
lengthAngle?: number;
lineWidth?: number;
label?: LabelRenderFunction;
labelPosition?: number;
labelStyle?: CSSProperties | ((dataIndex: number) => CSSProperties | undefined);
onBlur?: EventHandler<FocusEvent>;
onClick?: EventHandler<MouseEvent>;
onFocus?: EventHandler<FocusEvent>;
onKeyDown?: EventHandler<KeyboardEvent>;
onMouseOut?: EventHandler<MouseEvent>;
onMouseOver?: EventHandler<MouseEvent>;
paddingAngle?: number;
radius?: number;
reveal?: number;
rounded?: boolean;
segmentsShift?: number | ((dataIndex: number) => number | undefined);
segmentsStyle?: CSSProperties | ((dataIndex: number) => CSSProperties | undefined);
segmentsTabIndex?: number;
startAngle?: number;
style?: CSSProperties;
totalValue?: number;
viewBoxSize?: [number, number];
};
declare function PieChart(props: Props): JSX.Element;
}
I still think my response here has some base.
Cheers
from react-minimal-pie-chart.
Hi @mkarajohn,
I can't say for sure it's JetBrain. From a maintainer perspective I'd be inclined to take Typescript as a source of truth. If TS doesn't complain then it's fine.
User facing types are tested here and executed with bare tsc
from here. Would it be possible to reproduce the error outside the IDE?
As a next step we could get rid of React's defaultProps
(respecting size constraints of this library), but it's not on my top priority right now :) Happy to help out if you wanted to try a PR.
from react-minimal-pie-chart.
Well, it does in fact throw when simply running tsc
$ yarn tsc
yarn run v1.22.5
$ /mnt/4E141ED6141EC13F//Workspace//node_modules/.bin/tsc
src/pages/IssuesPage/subpages/Conflicts/components/Chart/Chart.tsx:155:12 - error TS2740: Type '{ key: string; background: string; css: SerializedStyles; lineWidth: number; data: never[]; }' is missing the following properties from type '{ animationDuration: number; animationEasing: string; center: [number, number]; data: Data; labelPosition: number; lengthAngle: number; lineWidth: number; paddingAngle: number; radius: number; startAngle: number; viewBoxSize: [...]; }': animationDuration, animationEasing, center, labelPosition, and 5 more.
155 <PieChart
~~~~~~~~
src/pages/IssuesPage/subpages/Conflicts/components/Chart/Chart.tsx:166:14 - error TS2740: Type '{ key: string; animate: true; animationDuration: number; animationEasing: string; background: string; css: SerializedStyles; lineWidth: number; data: ChartNormalizedData[]; }' is missing the following properties from type '{ animationDuration: number; animationEasing: string; center: [number, number]; data: Data; labelPosition: number; lengthAngle: number; lineWidth: number; paddingAngle: number; radius: number; startAngle: number; viewBoxSize: [...]; }': center, labelPosition, lengthAngle, paddingAngle, and 3 more.
166 <PieChart
~~~~~~~~
Found 2 errors.
I tried replicating here but I couldn't. https://codesandbox.io/s/optimistic-voice-mxfvs?file=/src/App.tsx
Honestly, I am not sure what is happening. For me it makes sense that it is throwing (wth the union of the Props
and DefaultProps
, also see this, the sandbox specifically) but if I cannot reproduce it in another environment, what can I say 😅
So, what should I do? Would a PR that undoes the union of the Props
and DefaultProps
be an acceptable approach? You tell me.
from react-minimal-pie-chart.
2 3 brief questions:
- Can you build your app?
- Is there a way to replicate the issue out of your machine?
- What's your IDE and which version?
from react-minimal-pie-chart.
Can you build your app?
No, the compiling process throws, since this is a TS error
Is there a way to replicate the issue out of your machine?
I tried in the codesanbox, above, but I cannot reproduce.
But it also happens on my Mac machine, not only on Linux
What's your IDE and which version?
On linux:
WebStorm 2020.3
Build #WS-203.5981.135, built on November 27, 2020
On Mac:
WebStorm 2020.2.4
Build #WS-202.8194.6, built on November 24, 2020
BTW, this i how my IDE sees the type when I hover over the component
export function PieChart( props: Props & {animationDuration: number, animationEasing: string, center: [number, number], data: Data, labelPosition: number, lengthAngle: number, lineWidth: number, paddingAngle: number, radius: number, startAngle: number, viewBoxSize: [number, number]}): JSX.Element
from react-minimal-pie-chart.
Do you maybe build through your IDE? I've just built the chart in a CRA with TS and no errors whatsoever.
from react-minimal-pie-chart.
Do you maybe build through your IDE? I've just built the chart in a CRA with TS and no errors whatsoever.
I build from within the IDE, yes, but the tsc
command whose output I posted above was executed from a standalone terminal, outside of the IDE.
Like I said, I am just as confused, but since I have found a workaround, I am not really requesting for a fix :)
from react-minimal-pie-chart.
Just one last thing: would you mind pasting here your tsconfig.json
file?
from react-minimal-pie-chart.
Sure, I don't mind even if it's not the last thing 😄
{
"compilerOptions": {
"target": "es5",
"lib": ["dom", "dom.iterable", "esnext"],
"baseUrl": "./src",
"allowJs": true,
"skipLibCheck": true,
"esModuleInterop": true,
"allowSyntheticDefaultImports": true,
"strict": true,
"forceConsistentCasingInFileNames": true,
"module": "esnext",
"moduleResolution": "node",
"resolveJsonModule": true,
"isolatedModules": true,
"noEmit": true,
"jsx": "react",
"downlevelIteration": true,
"noFallthroughCasesInSwitch": true
},
"include": ["src"]
}
The codesandbox from above uses the basic configurations I use on my machine
from react-minimal-pie-chart.
I found this thread when looking for a solution to this exact problem.
So the workaround is to override the TypeScript types? Seems broken. At least this issue should be re-opened.
I'm on TypeScript 4.1.2. Would really appreciate a fix for this. My workaround for now is to @ts-ignore
your type signature, which is hardly any good when the objective is to increase the strictness of the types everywhere.
from react-minimal-pie-chart.
Related Issues (20)
- Not possible to render content inside chart on 8.0 HOT 1
- Dependency Dashboard
- Custom SVG label is rendered on same position HOT 3
- Side parts of Dognut chart are squashed HOT 1
- Not able to add multiline text with different format in central of pie/donut HOT 1
- Edges are getting cut on hover, when we render Doughnut chart HOT 1
- Expose the Typescript Types 🙏 HOT 7
- Hide tooltip HOT 3
- Feature OnMouseOver or specific classname when it is selected HOT 2
- Svg is filling all the space and truncate when becomes larger HOT 1
- How to style labels ? HOT 2
- Update React dev dependencies to v18 HOT 1
- Label font color not changing HOT 1
- Segment Border HOT 1
- Segment custom attributes for event handling HOT 1
- Are there any ways to set the same distance/padding between slices HOT 1
- Make labelPosition support segmentIndex callback function HOT 1
- PieChart component rendering error ExceptionsManager Error: Text strings must be rendered within a <Text> component. HOT 1
- Console warnings when using SSR HOT 5
- Implementation Question HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from react-minimal-pie-chart.