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(types): don't overwrite types of infer fallback #2669

Merged
merged 4 commits into from
Dec 18, 2022

Conversation

CodyJasonBennett
Copy link
Member

@CodyJasonBennett CodyJasonBennett commented Dec 18, 2022

Addresses some type regressions with {} and children investigated in #2668.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 18, 2022

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 c3f541f:

Sandbox Source
example Configuration

@krispya
Copy link
Member

krispya commented Dec 18, 2022

This fixes my issue.

krispya added a commit to Nicetouchco/react-three-fiber that referenced this pull request Dec 18, 2022
@krispya
Copy link
Member

krispya commented Dec 18, 2022

Not to celebrate too soon, the nested instrinsics are giving me the error Type 'Element' is not assignable to type 'Object3D<Event> | (string & Object3D<Event>)'

For example:

 <mesh>
        <planeGeometry />
        <meshBasicMaterial>
          {children}
        </meshBasicMaterial>
      </mesh>

@CodyJasonBennett
Copy link
Member Author

Yeah, there's a myriad of issues going on here. If I can figure out how to narrow keys into a union that extend a type, I can fix it.

@CodyJasonBennett
Copy link
Member Author

CodyJasonBennett commented Dec 18, 2022

b657651 works for now.

I'd like WithMathProps to instead only map keys that extend the "math" signature as per my previous comment (and be renamed MathProps), but I haven't any luck. That'll also help clamp down noise from non-classes in ThreeElements that are just never. Referencing https://www.typescriptlang.org/docs/handbook/2/mapped-types.html#key-remapping-via-as.

type MathTypes = THREE.Color | MathRepresentation | VectorRepresentation
type MathKeys<P> = { [K in keyof P]-?: P[K] extends MathTypes ? never : K }[keyof P]

type MathType<M> = M extends THREE.Color
  ? ConstructorParameters<typeof THREE.Color> | THREE.ColorRepresentation
  : M extends MathRepresentation
  ? M extends VectorRepresentation
    ? M | Parameters<M['set']> | Parameters<M['setScalar']>[0]
    : M | Parameters<M['set']>
  : {}

type MathProps<P> = {
  [K in MathKeys<P>]: MathType<P[K]>
}

@krispya
Copy link
Member

krispya commented Dec 18, 2022

Thanks Cody. I can confirm this fixed the issues I was having. Drei's type issues dropped by a significant amount too.

krispya added a commit to Nicetouchco/react-three-fiber that referenced this pull request Dec 18, 2022
@CodyJasonBennett
Copy link
Member Author

Cool, if you can update the issue with a txt or snapshot that would help with withstanding issues.

@CodyJasonBennett CodyJasonBennett merged commit cec4297 into v9 Dec 18, 2022
@CodyJasonBennett CodyJasonBennett deleted the fix/infer-fallback-types branch December 18, 2022 18:16
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.

2 participants