-
Notifications
You must be signed in to change notification settings - Fork 30
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
feat: add cartesianProduct
function
#241
Conversation
@yamcodes thanks for the PR. some considerations.
export function product<T>(...arrays: T[]): T[][] {
export function product<T extends any[]>(...arrays: [...T]): Array<{[K in keyof T]: T[K][number]}>
export function product<T extends any[]>(...arrays: T): T[][] {
let out: T[][] = [[]]
for (const array of arrays) {
const result: T[][] = []
for (const currentArray of out) {
for (const item of array) {
const currentArrayCopy = currentArray.slice()
currentArrayCopy.push(item)
result.push(currentArrayCopy)
}
}
out = result
}
return out
} |
and since it is now possible to have better types, I recommend creating a test for the types, a suggestion: import * as _ from 'radashi'
describe('product types', () => {
test('with an empty array', () => {
const result = _.product([])
expectTypeOf(result).toEqualTypeOf<[never][]>()
})
test('with two arrays of the same type', () => {
const result = _.product(['red', 'blue'], ['fast', 'slow'])
const [[v1, v2]] = result
expectTypeOf(result).toEqualTypeOf<[string, string][]>()
expectTypeOf(v1).toEqualTypeOf<string>()
expectTypeOf(v2).toEqualTypeOf<string>()
})
test("with two arrays of different types", ()=> {
const result = _.product(['red', 'blue'], [1, 2])
const [[v1, v2]] = result
expectTypeOf(result).toEqualTypeOf<[string, number][]>()
expectTypeOf(v1).toEqualTypeOf<string>()
expectTypeOf(v2).toEqualTypeOf<number>()
})
test("with three arrays of different types", ()=> {
const result = _.product(['red', 'blue'], [1, 2], [true, false])
const [[v1, v2, v3]] = result
expectTypeOf(result).toEqualTypeOf<[string, number, boolean][]>()
expectTypeOf(v1).toEqualTypeOf<string>()
expectTypeOf(v2).toEqualTypeOf<number>()
expectTypeOf(v3).toEqualTypeOf<boolean>()
})
test("with constant arrays of different types", ()=> {
const result = _.product(['red', 'blue'] as const, [1, 2] as const)
const [[v1, v2]] = result
expectTypeOf(result).toEqualTypeOf<["red" | "blue", 1 | 2][]>()
expectTypeOf(v1).toEqualTypeOf<"red" | "blue">()
expectTypeOf(v2).toEqualTypeOf<1 | 2>()
})
test("with a mix of constant and non-constant types", ()=> {
const result = _.product(['red', 'blue'], [1, 2] as const)
const [[v1, v2]] = result
expectTypeOf(result).toEqualTypeOf<[string, 1 | 2][]>()
expectTypeOf(v1).toEqualTypeOf<string>()
expectTypeOf(v2).toEqualTypeOf<1 | 2>()
})
}) |
Thanks for the PR! I think naming the function |
Is pure genius. I've learned something new today because I had no idea you could spread generic types. Thank you, Marlon! @aleclarson Thanks! It's a pleasure to join your community and finally contribute! |
Guys, I've updated the PR to reflect your great suggestions. Let me know what you guys think of it as a whole! |
Suggestions were applied and I've also enforced that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hey! There's a new requirement for PRs that introduce new features. Without this requirement met, we won't be able to merge this. Note that this PR can still be included in a
|
cartesianProduct
cartesianProduct
function
Co-authored-by: Marlon Passos <[email protected]>
+ Also use `castArray` instead of `.slice()` for shallow copy
+ Add type tests + Update tests to support multiple types + Update doc to display multiple types
13f6d54
to
d99dd3b
Compare
Benchmark Results
Performance regressions of 30% or more should be investigated, unless they were anticipated. Smaller regressions may be due to normal variability, as we don't use dedicated CI infrastructure. |
I've improved the type signature, but otherwise all is good 👍 Thanks for the contribution, @yamcodes! Hope to see more from you in the future :) |
A stable release To install: pnpm add [email protected] ![]() |
Wonderful, thank you and happy to help, looking forward to contribute again 😃 |
Tip
The owner of this PR can publish a preview release by commenting
/publish
in this PR. Afterwards, anyone can try it out by runningpnpm add radashi@pr<PR_NUMBER>
.Summary
Add a
cartesianProduct
function for arrays.NOTE: Unfortunately, this only supports a single type. Mixed types are not fully supported, for example,_.product([[1,2],["hello","world"]])
will throw a type error. Fully mixed types are tricky to achieve in TypeScript for Cartesian Products, because of how things get "mixed up". In the above example, we know the result is composed of arrays like[1, "world"]
with type[number, string]
. I haven't found a good way to solve it. See discussion here: https://gist.github.com/ssippe/1f92625532eef28be6974f898efb23efThanks to @MarlonPassos-git we now support mixed types! ❤️
Related issue, if any:
For any code change,
Does this PR introduce a breaking change?
No
Bundle impact
src/array/cartesianProduct.ts
Footnotes
Function size includes the
import
dependencies of the function. ↩