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: Add declarations export to package.json #5762

Closed
wants to merge 3 commits into from

Conversation

csandman
Copy link

@csandman csandman commented Sep 29, 2023

This is an attempt to fix #5743, in which the module augmentation described in the docs no longer works in projects using "moduleResolution": "bundler" | "node16" | "nodenext". As far as I can tell, it's because projects using those settings look for the exports field in the package.json, which is more strict about which file paths can be accessed in the dependency package. I'm honestly not the most familiar with the interactions between TypeScript, ES Modules, and module augmentation (the entire node package ecosystem is hard to keep track of haha), but this fix did the trick for me when I tested it in my own project chakra-react-select by modifying the react-select package.json manually.

I know that @FabianFrank already had a similar PR up for this fix (#5744) but it wouldn't actually build because preconstruct saw the exports config as invalid. I was able to fix this by adding the following to the preconstruct config:

    "exports": {
      "extra": {
        "./dist/declarations/src/*": {
          "types": "./dist/declarations/src/*.d.ts"
        }
      }
    }

I extended the export statement to use a wildcard as well, because in my project I'm also extending the types for the MultiValue props, as well as the indicators props, so hopefully we can stick with that. I'm not exactly sure how the path structure is supposed to work, but doing it like this also allowed for augmenting the nested declarations like "react-select/dist/declarations/src/components/MultiValue".

Anyway, I'm really not sure if this is the best approach to fixing this issue, but users of my package have been having issues with this for a while now before I finally discovered that this must be the issue, so it would be great if a fix for this could get merged in.


On a side note, this could be an opportunity to simplify the import path people have to use with something like this:

    "./declarations/*": {
      "types": "./dist/declarations/src/*.d.ts"
    }

Which would allow people to simplify their module augmentation to this:

declare module "react-select/declarations/Select" {
  export interface Props<Option, IsMulti extends boolean, Group extends GroupBase<Option>> {
    // ...
  }
}

However, this would only work for those whose projects are actually using the exports field, which would create a divergence in the guide for how to accomplish adding custom props with module augmentation, so I figured it was probably best to keep them consistent for now.


EDIT:

I actually went ahead and made two example CodeSandboxes for the original issue, one highlighting how the module augmentation is broken with the current version of this package v5.7.5, and another with the version generated by this PR showing it fixed:

Both of these will appear fixed in the generated CodeSandboxes below.

@changeset-bot
Copy link

changeset-bot bot commented Sep 29, 2023

🦋 Changeset detected

Latest commit: 5908f99

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
react-select Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 29, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 5908f99:

Sandbox Source
react-codesandboxer-example Configuration
react-select broken module augmentation with moduleResolution="bundler" PR
react-select fixed module augmentation with moduleResolution="bundler" PR

@FabianFrank
Copy link

Nice! 👍 I’ll close my PR since this is definitely better with a working build.

@csandman csandman changed the title Add declarations export to package.json fix: Add declarations export to package.json Sep 29, 2023
@csandman
Copy link
Author

Nice! 👍 I’ll close my PR since this is definitely better with a working build.

Sounds good, hopefully this gets some attention! I tried using patch-package on React Select's package.json in my own project to fix this, but as far as I know that doesn't really work right on a package.json. Plus, other people could probably benefit from this change.

@FabianFrank
Copy link

I tried using patch-package on React Select's package.json in my own project to fix this, but as far as I know that doesn't really work right on a package.json.

It does work if you pass --exclude when creating the patch, e.g. patch-package react-select --exclude.

@csandman
Copy link
Author

It does work if you pass --exclude when creating the patch, e.g. patch-package react-select --exclude.

Yeah I was able to actually create the patch, but it didn't appear to be sticking around after I installed my package in an app. That approach would probably work for someone who has react-select as a direct dependency though, like a normal web app.

You can see what I'm talking about with this PR I attempted:

I don't understand why it's not working exactly, but this comment sort of explains what the package.json isn't included by default.

@csandman
Copy link
Author

csandman commented Oct 6, 2023

@JedWatson @lukebennett88 @Methuselah96 Could someone take a look at this? It's a really small change that I can't image any possible regressions existing for. And it's a big pain point at the moment for anyone using the newer moduleResolution TS config that also needs to augment the select types.

@xec xec mentioned this pull request Oct 9, 2023
@emmatown
Copy link
Collaborator

Thanks for the PR but these internal files aren't really intended to be public API and be augmented directly like this so I don't think this makes sense to merge.

The various types here can already be augmented by using the place where they're exported publicly. e.g. the Props type can already be augmented via react-select/base

import Select, { GroupBase } from "react-select";
import type {} from "react-select/base";

declare module "react-select/base" {
  export interface Props<
    Option,
    IsMulti extends boolean,
    Group extends GroupBase<Option>
  > {
    something?: string;
  }
}

<Select something="" />;

https://codesandbox.io/s/react-select-module-augmentation-with-moduleresolution-bundler-fixed-fxpczr?file=/src/App.tsx

@floriancargoet
Copy link

Thanks!
It seems the key to making it work with "react-select/base" is the empty import import type {} from "react-select/base";.
Without it, TS doesn't find the module for augmentation.

@emmatown If augmenting the internal file is not supported, the official docs should be updated because that's exactly what is recommended.
https://react-select.com/typescript#custom-select-props

@lukebennett88
Copy link
Collaborator

Good point @floriancargoet.
I've just opened a PR to update the docs:
#5776

@csandman
Copy link
Author

csandman commented Oct 16, 2023

Thanks for the PR but these internal files aren't really intended to be public API and be augmented directly like this so I don't think this makes sense to merge.

The various types here can already be augmented by using the place where they're exported publicly. e.g. the Props type can already be augmented via react-select/base

@emmatown Thanks for this info. I had actually tried a few different ways of making this work without the nested augmentation but didn't realize you could do it from react-select/base. I had tried doing it from the root react-select, but that didn't end up augmenting the internal props, only the exported type. And like @floriancargoet mentioned, the nested augmentation has been the recommended approach for a while, so getting that updated in the docs would be great. Thanks for the alternative PR @lukebennett88!

One other question though, can anyone explain why the empty import import type {} from "react-select/base"; is required for the types to be found properly? I saw this pattern a bit in my research (well I saw an empty export {} from ... more), and I'm curious why this is a thing. Is it new with these newer moduleResolution types?

@csandman
Copy link
Author

Also, I had one last question, with this new approach to the module augmentation, is it possible to augment any of the other prop types? For example, in my project, I'm extending the LoadingIndicatorProps interface to add some extra custom props to it. I'm doing this because my package replaces all of the internal react-select components, using Chakra UI components in their place. And I'm allowing my users to wrap them similarly to the core package allows for with the <components.LoadingIndicator> approach (except mine is <chakraComponents.LoadingIndicator>).

Here is an example of what I'm doing in my current version: https://github.com/csandman/chakra-react-select/blob/3d9c4fc95ecb9e72a2943e6d94f11612b9aa637d/src/module-augmentation.ts#L227-L287

I tried augmenting all of the more specific component types from react-select/base, but it appears that none of the specific component types are being used from here, so that's not possible. Is there an alternative approach that I'm missing, or is the idea that these types are just not meant to be modified?

@emmatown
Copy link
Collaborator

One other question though, can anyone explain why the empty import import type {} from "react-select/base"; is required for the types to be found properly?

It's just to include the module in the TypeScript project so TypeScript is aware of it and it can be augmented. Any other sort of import of react-select/base would also work fine

Also, I had one last question, with this new approach to the module augmentation, is it possible to augment any of the other prop types? For example, in my project, I'm extending the LoadingIndicatorProps interface to add some extra custom props to it.

LoadingIndicatorProps and others are exported from the root at react-select so you can do this:

declare module "react-select" {
  export interface LoadingIndicatorProps<
    Option = unknown,
    IsMulti extends boolean = boolean,
    Group extends GroupBase<Option> = GroupBase<Option>
  > {
    something: string;
  }
}

@emmatown emmatown closed this Oct 17, 2023
@csandman
Copy link
Author

It's just to include the module in the TypeScript project so TypeScript is aware of it and it can be augmented. Any other sort of import of react-select/base would also work fine

Gotcha, thanks for the reply! That makes sense.

LoadingIndicatorProps and others are exported from the root at react-select so you can do this:

Wow, can't believe I missed this. I could have sworn I had tried this in the past and it didn't work, but maybe I didn't. Either way, thanks a lot for the solution! Seems like all of my problems are solved!

@csandman csandman deleted the patch-1 branch October 17, 2023 15:44
@floriancargoet
Copy link

Another tip for anyone landing on this issue:
If you're building a library, the empty import might be removed by your bundler, and the apps consuming your lib & .d.ts wouldn't see your augmentation.
That's what happened to me. I've solved it by exporting the type, ensuring it wouldn't be removed.

export type { Props as ReactSelectProps } from "react-select/base";

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Module augmentation not working with moduleResolution: bundler
5 participants