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

Add type for injectableStore #36

Closed
peytondmurray opened this issue Oct 7, 2022 · 2 comments
Closed

Add type for injectableStore #36

peytondmurray opened this issue Oct 7, 2022 · 2 comments
Assignees
Labels
feature New feature or request

Comments

@peytondmurray
Copy link
Contributor

Problem

The JupyterContextType contains an injectableStore entry, which is the redux store for the jupyter-react. The type of injectableStore is currently any, which bypasses static type checking and triggers errors when linted with tslint:

  • @typescript-eslint/no-unsafe-assignment: If you use object destructuring on the useJupyter hook to access injectableStore
  • @typescript-eslint/no-unsafe-member-access: If you try to call injectableStore.dispatch()
  • @typescript-eslint/no-unsafe-call: If you try to call injectableStore.dispatch()

Proposed Solution

  • Add a type to the injectableStore. In Store.ts, could we have
export type AppStore = typeof injectableStore;

?

@echarles
Copy link
Member

echarles commented Feb 1, 2023

To avoid users of this library to have to import a lot of jupyterlab packages while initialising the Redux store, a injectable store has been implemented to allow to lazy construct that Redux store. If we type injectableStore, the initial bundle will have to pull a lot of jupyterlab packages, which will weight a lot in the initial application loading.

We can think on how to tackle that to give a better experience, so let's leave this issue open for further discussion.

@echarles echarles self-assigned this May 16, 2023
@echarles echarles added the feature New feature or request label May 16, 2023
@echarles
Copy link
Member

injectableStore is now typed as InjectableStore, done in 469dfe7#diff-8fbf114973d6ded92d50de4776da769a050b603b19bd3a5d463f2f8276356dee

Closing. Thx @peytondmurray

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants