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

[STRIPES-859] Re-export type declarations from stripes-types #208

Closed
wants to merge 7 commits into from

Conversation

ncovercash
Copy link
Member

@ncovercash ncovercash commented Apr 19, 2023

Jira STRIPES-859

This PR adds index.d.ts files to re-export types stored in stripes-types. Initially, this consists of typings for stripes-components, stripes-core, stripes-final-form, and stripes-smart-components — all other modules will have no associated type information, as it was previously.

For convenience, here is a link to stripes-types.

What is in stripes-types so far?

For modules stripes-components, stripes-core, and stripes-smart-components, all declarations are denoted as any, disabling type checking and having no effect, e.g.:

export const RootContext: any;
export const withRoot: any;
export const CalloutContext: any;
export const useCallout: any;
export const stripesShape: any;
export const withStripes: any;
export const useStripes: any;
...

All exports from the original modules' index.js files are included here.

stripes-final-form is the exception to this; since it only has one export, we have created actual type definitions for the stripesFinalForm function.

What effect will this PR have?

First off, this will only affect TypeScript modules. Modules which do not use TS will not use the TS compiler and thus these type declaration files are irrelevant.

There are (as far as I know) only two modules which use TypeScript: ui-calendar and ui-bulk-edit, both of which contain their own type declarations for these modules (calendar, bulk-edit) which take precedence over any we specify here. Therefore, we can safely develop these types without breaking any builds; these will only be relied upon downstream once the overridden definitions are removed (which will happen after this PR is merged and additional type development).

@zburke @JohnC-80 if you would please review this when you're able!

@github-actions
Copy link

github-actions bot commented Apr 25, 2023

Jest Unit Test Statistics

0 tests  ±0   0 ✔️ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ±0 

Results for commit 78a413b. ± Comparison against base commit 33dd377.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Apr 25, 2023

BigTest Unit Test Statistics

0 tests  ±0   0 ✔️ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ±0 

Results for commit 78a413b. ± Comparison against base commit 33dd377.

♻️ This comment has been updated with latest results.

Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

We need to sort out why GitHub can't resolve @folio/stripes-types, and decide where @types/react belongs in our dependency hierarchy for this can be merged.

package.json Outdated Show resolved Hide resolved
components/index.d.ts Show resolved Hide resolved
@ncovercash ncovercash requested a review from zburke April 26, 2023 16:06
.eslintrc Show resolved Hide resolved
@ncovercash
Copy link
Member Author

@zburke would we be able to get this merged soon? I'd like to start using these typings in ui-calendar; I've improved a good few of the types, but copying them back into ui-calendar would break some things with ui-bulk-edit (since both would be declaring different types for stripes-components, as happened a few weeks ago)

@ncovercash
Copy link
Member Author

@JohnC-80 / @zburke thanks for the reviews; I think there's only one thing left before this can get merged then:

React typings are provided by a package @types/react; modules using these types would need to have the same version of @types/react, otherwise very weird things occur (for example, if my ui-module with @types/react@17.0.1 uses component <Button> from here with @types/react@17.0.2, it will fail to compile; the UI module expects the <Button> to be a React.ComponentType, which it is, but it's the wrong ComponentType since it doesn't strictly equal). I'm not an expert in yarn's dependency resolution by any means, and especially not when it comes to peer dependencies — what would be the best way to accomplish this?

@zburke
Copy link
Member

zburke commented Sep 19, 2023

Merged #211 instead

@zburke zburke closed this Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants