Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

Add d.ts files to built artifacts #6

Merged
merged 6 commits into from
Jul 3, 2018
Merged

Add d.ts files to built artifacts #6

merged 6 commits into from
Jul 3, 2018

Conversation

cdoremus
Copy link
Contributor

@cdoremus cdoremus commented Jun 10, 2018

This PR creates type definition (d.ts) files during the build (npm run build). This will allow the use of TypeScript for code-completion and error checking by applications that use this library.

The PR also includes some changes to the components to avoid naming conflicts. Most of these were changing the Props and State interface names to XXXXXProps and XXXXXState where XXXXX is the name of the component.

This work is being done to satisfy issue #5

As a side note, although modifying the build to create d.ts files turned out to be a simple process (just add declaration: true to tsconfig.json), there is very little documentation out there how this is done and a lot of misleading documentation that is old or only applicable to creating types for JavaScript files.

@cdoremus cdoremus requested a review from nickoneill June 10, 2018 16:13
@nickoneill
Copy link
Member

Thanks! This looks fantastic.

Two questions:

  • What are the @ts-ignore lines for?
  • I understand the naming conflicts, but it seems strange that this would export non-exported types like Props and State that are scoped to the file within the project. Maybe there's a flag to ignore unexported types?

@cdoremus
Copy link
Contributor Author

The props and state need to be exported to satisfy the TS compiler. But any member can be excluded from the d.ts file by using the @internal annotation in a comment above its definition and adding stripInternal: true to tsconfig.json.

I would think that props would need to be exported so that clients of a component would know the component prop names and their types, but I think that the state interface should be designated as internal.

@nickoneill nickoneill merged commit 2964d53 into master Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants