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

Typescript: Props aren't exported anymore #1979

Closed
dattebayorob opened this issue Apr 23, 2021 · 9 comments
Closed

Typescript: Props aren't exported anymore #1979

dattebayorob opened this issue Apr 23, 2021 · 9 comments
Assignees
Labels
Type: Bug Issue contains a defect related to a specific component.
Milestone

Comments

@dattebayorob
Copy link

dattebayorob commented Apr 23, 2021

[x] bug report
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://forum.primefaces.org/viewforum.php?f=57

Codesandbox Case (Bug Reports)

https://codesandbox.io/s/optimistic-maxwell-kz1vz?file=/src/App.tsx

[ Edit ]
Just tested, and using "declare namespace" types aren't exported at all.
Using only exports worked: https://gist.github.com/dattebayorob/689b3a8382ea8cad5cf9b3a748d89c79

@mertsincan mertsincan self-assigned this Apr 23, 2021
@mertsincan mertsincan added the Status: Discussion Issue or pull request needs to be discussed by Core Team label Apr 23, 2021
@mertsincan
Copy link
Member

mertsincan commented Apr 23, 2021

Hi @dattebayorob,

I have rewritten all d.ts files to suggest more specific property's values ​​to Users. This topic is about declare namespace. In the next version, I can change the declare namespace with the declare module.

declare namespace Button {
    export interface ButtonProps {...}
}

import { Button } from "primereact/button";
const props: Button.ButtonProps;

OR

declare module 'primereact/button' {
    export interface ButtonProps {...}
}

import { ButtonProps } from "primereact/button";
const props: ButtonProps;

But my personal opinion is to use Button.ButtonProps.

WDYT?

@dattebayorob
Copy link
Author

dattebayorob commented Apr 23, 2021

Ummmm Understood, I think it's great @mertsincan , but in this case the version is a Break version.
Everyone who use proxies on components will be affected by this change.

In my case:

import {
Calendar as PrimeCalendar,
CalendarProps as PrimeCalendarProps
} from 'primereact/calendar';

export interface CalendarProps extends Omit<PrimeCalendarProps, 'minDate'>, FormControlLabelProps, FormControlErrorProps {
name: string;
minDate?: Date | null;
}

export const Calendar = (props: CalendarProps) => {
const { field, ref, error } = useFormField<Date | Date[], HTMLInputElement>({ name: props.name });
return <BaseCalendar { ...field } { ...props } error={error} inputRef={ref} />
}

@mertsincan
Copy link
Member

Ok, I'll add the Breaking Changes section to Changelog.md about this.
Feedback from users is important in this regard. Which one would you prefer for props or other type definition?

  1. props: Calendar.CalendarProps
    Or
  2. props: CalendarProps

@dattebayorob
Copy link
Author

dattebayorob commented Apr 23, 2021

I think the second option is better for compositions

import type { CalendarProps } from 'primereact/calendar'; //Only type definition is imported, no code generated after transpile

(...)

[ Edit ]
import type { Calendar } from 'primereact/calendar' works as well...

@master117
Copy link
Contributor

I had raised the same Issue in another location.

I also believe the second version to be better for composition types. I'm a it disappointed you can't deep import in JS. Like this

import { Button.ButtonProps as ButtonProps } from "primereact/button";

How about the option to make it like this?

import { ButtonProps } from "primereact/button/buttonprops";

@cailenmusselman
Copy link

Just upgraded and my project broke as well 😢
@mertsincan - For option 1: props: Calendar.CalendarProps
Would that work? ToastMessage is currently being exported from namespace Toast and when I try
let message: Toast.ToastMessage;
I get the following TS error:
'Toast' only refers to a type, but is being used as a namespace here. TS2702

IIRC I'm using the default typescript settings that are used by create-react-app

@mertsincan mertsincan added Type: Bug Issue contains a defect related to a specific component. and removed Status: Discussion Issue or pull request needs to be discussed by Core Team labels Apr 24, 2021
@mertsincan mertsincan added this to the 6.3.1 milestone Apr 24, 2021
@mertsincan mertsincan changed the title Typescript: Props aren't exported anymore after 6.3.0 Typescript: Props aren't exported anymore Apr 24, 2021
@mertsincan
Copy link
Member

Thank you very much for all the feedback! I made some changes again. On 6.3.1;

import {CalendarProps} from 'primereact/calendar';
import {ToastMessage} from 'primereact/toast';

How about the option to make it like this?
import { ButtonProps } from "primereact/button/buttonprops";

Thanks a lot for your suggestion, @master117 ;) But, it seems like primereact/component will be better.

@mertsincan
Copy link
Member

Hi,

6.3.1 Released. Could you please try it?

@dattebayorob
Copy link
Author

Great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a defect related to a specific component.
Projects
None yet
Development

No branches or pull requests

4 participants