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(create-injection-token): allow provideFn to accept factory #100

Merged
merged 1 commit into from
Oct 7, 2023

Conversation

nartc
Copy link
Collaborator

@nartc nartc commented Oct 6, 2023

Closes #99

No docs yet because my brain is not firing on all cylinders this morning (Insomnia struck last night so I've been up since 2AM). I'll add docs later or @wanoo21 can put some docs 😋.

cc @wanoo21 no docs so you might want to take a look at the new tests in this PR for usage.

For "optional factory" case, I had to add a new function createNoopInjectionToken() instead. Typing is tricky with the optional factory.

// "multi" token; unfortunately until I can acquire better typescript skill, we have to duplicate "true" here
const [injectFn, provideFn] = createNoopInjectionToken<number, true>('description', { multi: true });

injectFn(); // number[]
provideFn(1); // accepts number
provideFn(() => 1); // accepts a factory returning a number

// normal token
const [injectFn, provideFn] = createNoopInjectionToken<number>('description');
injectFn(); // number;

@nartc nartc marked this pull request as draft October 6, 2023 13:28
@nx-cloud
Copy link

nx-cloud bot commented Oct 6, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 610d14b. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 4 targets

Sent with 💌 from NxCloud.

@nartc nartc force-pushed the fix/injection-token-provide-factory branch from 90f60a9 to 13d9bbe Compare October 6, 2023 13:57
@nartc nartc marked this pull request as ready for review October 6, 2023 14:02
@nartc nartc force-pushed the fix/injection-token-provide-factory branch from 13d9bbe to 0082a07 Compare October 6, 2023 14:02
@nartc nartc requested review from eneajaho and tomalaforge October 6, 2023 14:02
@nartc nartc self-assigned this Oct 6, 2023
@nartc nartc force-pushed the fix/injection-token-provide-factory branch from 0082a07 to 610d14b Compare October 6, 2023 14:05
@wanoo21
Copy link
Contributor

wanoo21 commented Oct 6, 2023

So fast, thanks @nartc, I'll check the source code and test examples this weekend, and I sure can help with docs!

You should take a break, hahaha!

@nartc
Copy link
Collaborator Author

nartc commented Oct 6, 2023

Waiting on @wanoo21 confirmation before merging.

@wanoo21
Copy link
Contributor

wanoo21 commented Oct 7, 2023

I think we're good to go. Let's merge it and I'll update the docs before release.

Should I create a new issue for documentation?

@nartc nartc merged commit 55f31b3 into main Oct 7, 2023
@nartc nartc deleted the fix/injection-token-provide-factory branch October 7, 2023 04:49
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.

[RFC] createInjectionToken generic type and optional factory
3 participants