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

[v9] fix: Euler types and ReactProps #2705

Merged
merged 1 commit into from
Jan 13, 2023

Conversation

krispya
Copy link
Member

@krispya krispya commented Jan 13, 2023

This does three things to fix types.

  1. Adds an export to ReactProps to fix error TS4023: Exported variable {ComponentName} has or is using name 'ReactProps' from external module "C:/dev/nt--drei/node_modules/@react-three/fiber/dist/declarations/src/three-types" but cannot be named.
    Is this a bug with lerna multi package project with TS4053 error? microsoft/TypeScript#40718
    XXX has or is using name 'Foo' from external module "../bar" but cannot be named microsoft/TypeScript#9944

  2. Makes MathRepresenation a union with THREE.Euler since it's optional order string parameter throws a wrench in any algorithmic detection as far as I can see.

  3. Adds a MathType Euler export so ReactThreeFiber.Euler works.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 13, 2023

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 56a11a9:

Sandbox Source
example Configuration

@krispya krispya closed this Jan 13, 2023
@krispya krispya reopened this Jan 13, 2023
@krispya krispya changed the base branch from master to v9 January 13, 2023 09:43
@CodyJasonBennett
Copy link
Member

CodyJasonBennett commented Jan 13, 2023

Would it be error prone to loosen MathRepresentation to { set(...args: any[]): any } ? I can only think of TypedArrays.

Comment on lines -32 to +33
interface ReactProps<P> {
export interface ReactProps<P> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW how were you getting error TS4023: Exported variable {ComponentName} has or is using name 'ReactProps' from external module "C:/dev/nt--drei/node_modules/@react-three/fiber/dist/declarations/src/three-types" but cannot be named.? Is this with PNPM and symlinks?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is with the Drei repo in the base src folder using yarn and no symlinks (afaik). There must be a better solution than this, but this worked. :)

@krispya
Copy link
Member Author

krispya commented Jan 13, 2023

Would it be error prone to loosen MathRepresentation to { set(...args: any[]): any } ? I can only think of TypedArrays.

Wouldn't this mean any instance with a set method passes the math representation test now? There are a few floating around three, such as the Raycaster.

@krispya krispya mentioned this pull request Jan 13, 2023
@CodyJasonBennett
Copy link
Member

I'm happy to merge this. I do suspect a similar issue with #2668 (comment).

@krispya
Copy link
Member Author

krispya commented Jan 13, 2023

I think so too. Okay, let's merge and make adjustments going forward. I'll see if we can rollback exporting ReactProps once all the type systems are working as we expect.

@CodyJasonBennett CodyJasonBennett changed the title fix: Euler types and ReactProps [v9] fix: Euler types and ReactProps Jan 13, 2023
@CodyJasonBennett CodyJasonBennett merged commit 8170527 into pmndrs:v9 Jan 13, 2023
@krispya krispya deleted the fix/euler-types branch March 15, 2023 00:46
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