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

[wip] Make REPL URL params extensible #525

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

bollwyvl
Copy link
Collaborator

@bollwyvl bollwyvl commented Feb 28, 2022

References

Code changes

this is still pretty WIP, names, etc. will probably change a lot.

  • adds IReplApi as an exposed service in a new plugin
    • offers some before/after events for a few points in the app lifecycle
  • moves existing URL params into separate plugins
    • register with partial schema
    • dynamically update themes, kernels, etc.
    • use the schema for validation, defaults
      • it's rather hard to reason about this once pluggable, as there's no way to know when all of the plugins would be done
  • tests
  • add a doc test app that actually uses the schema (maybe as a notebook with iframes)
    • add some headers to jupyter lite serve to allow re-embedding self?
    • add localforage-memoryStorageDriver to allow iframes, because other drivers don't work?
    • add postMessage API to get access to schema

User-facing changes

  • n/a

Backwards-incompatible changes

  • n/a

Notes

  • the theme plugin acts a little funny, and calls itself very early
    • some race condition between the theme and the ResponsiveToolbar leads to it collapsing at weird times (it has a 500ms debouncer that's hard to reason about)
    • the ... button that reveals the toolbar is actually really awesome, but doesn't quite do the right thing

@github-actions
Copy link
Contributor

lite-badge 👈 Try it on ReadTheDocs

@bollwyvl bollwyvl added app:repl enhancement New feature or request labels Feb 28, 2022
@jtpio jtpio added this to the 0.1.0 milestone Mar 1, 2022
@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Mar 4, 2022

Will probably block on this until #528 is ready, as would like to do the test app with rjsf.

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Mar 4, 2022

I kinda wonder if our tests fail on windows because them temp dir is too long to delete...

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Mar 5, 2022

The more I've been tinkering with this, the more I'm thinking:

  • this should apply to more of the apps
  • it should centralize even more of the URL params (e.g. room, path, etc.)
  • there are some challenges with race conditions

So: maybe this should hang off IRouter

  • and probably upstream, eventually

So in the near term, I might split off some of the enablers of this work which are orthogonal to the actual param API:

  • the localforage-memory
  • extra headers for jupyter lite serve

@jtpio
Copy link
Member

jtpio commented Mar 8, 2022

  • this should apply to more of the apps

  • it should centralize even more of the URL params (e.g. room, path, etc.)

  • there are some challenges with race conditions

This sounds good. It would be better for the existing parameters like room and path to also be properly specified.

@bollwyvl bollwyvl mentioned this pull request Mar 12, 2022
7 tasks
@jtpio jtpio mentioned this pull request Apr 29, 2022
@jtpio jtpio modified the milestones: 0.1.0, 0.2.0 Mar 14, 2023
@jtpio jtpio mentioned this pull request Apr 5, 2023
@jtpio jtpio removed this from the 0.2.0 milestone Sep 28, 2023
@jtpio jtpio added this to the 0.3.0 milestone Sep 28, 2023
@jtpio jtpio modified the milestones: 0.3.0, 0.4.0 Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app:repl enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants