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

expose Dictionary as part of an API #1118

Merged
merged 1 commit into from
Jun 11, 2018
Merged

expose Dictionary as part of an API #1118

merged 1 commit into from
Jun 11, 2018

Conversation

alex-okrushko
Copy link
Member

When code exports a symbol whose type is inferred (or contains an inferred type), the type must be imported so that a correct name can be generated for it in the resulting .d.ts file.

For example, that means when we create any selector we have to import MemoizedSelector to the file, and, also not to have the unused import linter error, at least one of the selectors has to be fully typed.

Since MemoizedSelector is exported through public_api.ts, solving this problem is pretty straightforward, however one of the teams has started using entities now and has to import Dictionary for the exactly same reason, but Dictionary is not exported.

e.g. this is the code that they need to use:

export const fooEntities: MemoizedSelector<StateWithFoo, Dictionary<Foo>> =
    createSelector(fooFeature, adapter.getSelectors().selectEntities);

for more on exported variables with inferred type:
microsoft/TypeScript#5711 (comment)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 87.971% when pulling 58ae699 on alex-okrushko:master into 550b67e on ngrx:master.

@brandonroberts
Copy link
Member

Will you remove the serializer commit from this PR since you have another one open for that?

@alex-okrushko
Copy link
Member Author

Ah, yeah, not sure how that one got here as well...

@alex-okrushko
Copy link
Member Author

@brandonroberts Done.

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.

3 participants