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

@uppy/core state and store types are outdated #4090

Closed
2 tasks done
a-kriya opened this issue Sep 6, 2022 · 7 comments · Fixed by #4477
Closed
2 tasks done

@uppy/core state and store types are outdated #4090

a-kriya opened this issue Sep 6, 2022 · 7 comments · Fixed by #4477
Assignees
Labels
Bug Types Issues relating to the Typescript definition files

Comments

@a-kriya
Copy link
Contributor

a-kriya commented Sep 6, 2022

Initial checklist

  • I understand this is a bug report and questions should be posted in the Community Forum
  • I searched issues and couldn’t find anything (or linked relevant results below)

Link to runnable example

No response

Steps to reproduce

  • Access the added files with uppy.state.files, validated by TS
  • Error: cannot read property "files" of undefined

Expected behavior

TS to error out when accessing uppy.state and instead support uppy.store.state.

Actual behavior

state is nested under store:

image

@Murderlon
Copy link
Member

AFAIK this is intended and you should never access the state like that. Instead, use uppy.getState (or more specific ones, like getFiles).

@Murderlon Murderlon closed this as not planned Won't fix, can't repro, duplicate, stale Sep 8, 2022
@a-kriya
Copy link
Contributor Author

a-kriya commented Sep 8, 2022

@Murderlon The methods cannot be made reactive while the object can (as seen in the screenshot).

To replace it with methods will involve events instead, which is currently not ideal as you have noted in another ticket.

And regardless of this being intended for external usage, the typings still seem incorrect.

The documentation is also in need of an update, I think: https://uppy.io/docs/uppy/#store-defaultStore

@Murderlon
Copy link
Member

The methods cannot be made reactive while the object can (as seen in the screenshot).

What do you mean with this? What are you trying to do?

To replace it with methods will involve events instead, which is currently not ideal as you have noted in another ticket.

I'm not sure I know which ticket, what is the downside here?

And regardless of this being intended for external usage, the typings still seem incorrect.

TS to error out when accessing uppy.state and instead support uppy.store.state.

I think uppy.state is not typed currently, which means it should already error?

@a-kriya
Copy link
Contributor Author

a-kriya commented Sep 8, 2022

What do you mean with this? What are you trying to do?

I use Uppy inside a Vuex module. The Uppy instance is added to the state, which causes Vue to observe changes within it. This allows me to write simple getters that automatically update on Uppy's state changes:

{
  numberFiles(): number {
    return Object.values(uppy.state.files).length
  },
  totalSize(): number {
    return Object.values(uppy.state.files).reduce((size, file) => size + file.size, 0)
  }
}

The above was possible before, but since the update, the typings say that uppy.state exists, but that's wrong. It's uppy.store.state now, and TS errors out when I write that.

I'm not sure I know which ticket, what is the downside here?

Some issues that are described in #3248 and #3938.

@Murderlon Murderlon reopened this Sep 9, 2022
@Murderlon Murderlon changed the title Uppy class typings outdated @uppy/core state and store types are outdated Sep 9, 2022
@Murderlon Murderlon added Types Issues relating to the Typescript definition files Core and removed Triage labels Sep 9, 2022
@Murderlon
Copy link
Member

Thanks for explaining. I think the readonly state type was forgotten to be removed in the latest major version, in which the deprecated state property was removed from the class.

@a-kriya
Copy link
Contributor Author

a-kriya commented May 26, 2023

@aduh95 #4477 just removes the typing for the older state property, but store still isn't typed for the Uppy instance when it does exist:

image

@aduh95 aduh95 reopened this May 26, 2023
@Murderlon
Copy link
Member

Uppy has been rewritten in TypeScript and the 4.0 release contains all type improvements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Types Issues relating to the Typescript definition files
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants