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

remove deps to phosphorjs #6501

Open
akosyakov opened this issue Nov 6, 2019 · 8 comments
Open

remove deps to phosphorjs #6501

akosyakov opened this issue Nov 6, 2019 · 8 comments
Labels
dependencies pull requests that update a dependency file proposal feature proposals (potential future features) shell issues related to the core shell

Comments

@akosyakov
Copy link
Member

akosyakov commented Nov 6, 2019

phosphor.js was archived: https://github.com/phosphorjs/phosphor#phosphorjs

We don't really use much from there except widgets. Also it's causing issues since phosphor.js comes with own concepts for events (signals), does not support disposables and does not live in DI context.

Instead of maintaining our own fork, it would be better to copy relevant bits, refactor them to use our events, clean up its dependencies, put in DI context and then clean up the shell. It would reduce amount of dependencies and size of bundled code as well.

@akosyakov akosyakov added dependencies pull requests that update a dependency file proposal feature proposals (potential future features) shell issues related to the core shell labels Nov 6, 2019
@kittaakos
Copy link
Contributor

It would reduce amount of dependencies and size of bundled code as well.

It sounds great, but would it worth the effort? The @phosphor folder is 1.2 MB compared to the ~500 MB size of the final electron application. Even if we do the refactoring and API adjustments, we still wouldn't be able to use async API for menus, for instance, and factories for DockPanel.

@akosyakov
Copy link
Member Author

The @phosphor folder is 1.2 MB compared to the ~500 MB size of the final electron application.

For the browser context, we don't have electron and there less code we bundle less time a user waits to get an IDE.

Even if we do the refactoring and API adjustments, we still wouldn't be able to use async API for menus, for instance, and factories for DockPanel.

Not sure what you mean. If we own code, we can change it as we like.

@BroKun
Copy link
Member

BroKun commented Nov 15, 2019

I totally agree with you that we are much better at events and DI than Phosphor. If we want to achieve a better experience than vscode, we need to make a lot of improvements, such as more flexible split-panel. The existing implementation of Phosphor may become a constraint.

@akosyakov
Copy link
Member Author

I think it is also better instead of using https://github.com/jupyterlab/lumino, since it will make it possible to integrate JLab widgets in our widget without version conflicts.

@mikael-desharnais
Copy link
Contributor

Hi
this can be quite an issue at the moment.
Should we go on developing extensions that use the Phosphor lib & API ? or should we add a new library that would provide these features ?
Could you give us some hints on the solution that is the most likely to be followed ?
Regards,

@akosyakov
Copy link
Member Author

@mikael-desharnais you can use phorpshor.js for now, even if we merge it we will keep API the same just change namespaces.

@thegecko
Copy link
Member

It's not clear the effort involved switching to lumino, but assuming it's a lot of work, other options could be:

  • Hard fork the repo and maintain our own widget framework we control ourselves (as suggested in the README)
  • Take on ownership of phosphorjs and continue it's ideals (we would need to contact the original owner for permission and access to the @phosphor namespace on npm)

@JonasHelming
Copy link
Contributor

@tsmaeder : We checked our projects. Most do not directly use Phosphor except one where we do "Tabs in Tabs" and use like ResizeMessage, custom layouts, or similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies pull requests that update a dependency file proposal feature proposals (potential future features) shell issues related to the core shell
Projects
None yet
Development

No branches or pull requests

6 participants