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

Make INotebookTracker parameter mandatory in TOC extension, as it had been #11664

Conversation

JasonWeill
Copy link
Contributor

References

Affects issue #11663. A partial rollback of changes made in #11445.

Code changes

Reinstates INotebookTracker as a mandatory parameter in the TOC extension, as without a notebook tracker, the TOC extension displays nothing.

User-facing changes

Required to make TOC extension functional in RetroLab. Should have no effect on JupyterLab.

Backwards-incompatible changes

@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@jtpio
Copy link
Member

jtpio commented Dec 11, 2021

Thanks @jweill-aws for looking into this.

Unfortunately this won't fix the Table of Contents being empty. As mentioned in jupyterlab/retrolab#275 (comment), RetroLab does include the plugin that provides INotebookTracker on the notebook page. So even if INotebookTracker is marked as optional in the plugin definition of the JupyterLab package, it will still be provided and available in RetroLab. So this won't change anything.

Instead we need to rework the TOC extension so it's more modular, and decouple the TableOfContents widget from ITableOfContentsRegistry. #11530 provides more context about this, but still need to be finished. Also this will probably have to be a 4.0 change.


As a general guideline, it's better to not make services requires when they don't have to. Instead making them optional allow for greater plugin re-usability and this is what made it possible to build RetroLab in the first place. If we make them requires like this change suggests, it will be more difficult for other lab-based applications to reuse the plugins as is. So #11445 is still a good change and a general pattern to follow.

As mentioned elsewhere, we can imagine a simple lab-based app with a code editor on the right and the table of contents on the left side of the page. This app would not provide the INotebookTracker but will still want to use the functionalities of the TOC extensions depending on IEditorTracker for example. If INotebookTracker is required, the TOC plugin will fail to activate and it will not be possible to reuse any functionality of the plugin.

@jtpio
Copy link
Member

jtpio commented Dec 13, 2021

For more information about requires and optional: https://jupyterlab.readthedocs.io/en/latest/extension/extension_dev.html#tokens

@JasonWeill JasonWeill closed this Jan 28, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants