feat(types): allow typed access to properties added to entry #4814
Conversation
Haroenv
requested review from
eunjae-lee,
a team and
tkrugg
and removed request for
a team
|
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 3517505:
|
Haroenv
changed the title
Haroenv
changed the title
| version: string; | ||
|
|
||
| // @major remove these in favour of the exports | ||
| /** @deprecated */ |
There was a problem hiding this comment.
@francoischalifour suggested not deprecating these yet, as we aren't sure where the export should best be from. At the moment it's /es/helpers, but that could change?
Personally I'm more of the opinion to deprecate them with /es/helpers as a workaround, still leaving the option open for a more thorough review of what's exported where in a major version
wdyt @tkrugg / @shortcuts
There was a problem hiding this comment.
As long as we have clear warnings etc., I don't see any issue deprecating them
Haroenv
force-pushed
the
feat/export-helpers
branch
from
b9d64f0
to
60d3d14
Compare
also removes the file for "main" as it's inconsistent with index.es.ts
Haroenv
force-pushed
the
feat/export-helpers
branch
from
60d3d14
to
2e2d270
Compare
Haroenv
changed the title
| version: string; | ||
|
|
||
| // @major remove these in favour of the exports | ||
| /** @deprecated */ |
There was a problem hiding this comment.
As long as we have clear warnings etc., I don't see any issue deprecating them
|
The thing is we'll likely update/clean our exports during the React InstantSearch migration, so I'm not sure that's the right time for this change (also definitely an improvement) because it will make it harder to not introduce breaking changes. |
|
Changing exports is always a breaking change @francoischalifour, right? I know for sure that we don't want to continue exposing the helpers as a property on the es entry point (it's not documented), the deprecation message mentions /es/helpers, as I don't think it's useful to discourage use without explaining what a right way of using it is. This PR mainly focuses on having the right types for something that already is the case, whether it's typed or not, removing those entries is a breaking change :) |
|
Sorry I got confused and forgot that the PR changed from updating the exports to only updating typings—all good then! |
Summary
The helpers in the esm entry file were attached to the "prototype" of instantsearch instead of exported, which makes them non-treeshakeable. Removing it now would be a breaking change, but that form can at least already be encouraged.
Result