-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Comments
It sounds great, but would it worth the effort? The |
For the browser context, we don't have electron and there less code we bundle less time a user waits to get an IDE.
Not sure what you mean. If we own code, we can change it as we like. |
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. |
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. |
Hi |
@mikael-desharnais you can use phorpshor.js for now, even if we merge it we will keep API the same just change namespaces. |
It's not clear the effort involved switching to lumino, but assuming it's a lot of work, other options could be: |
@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. |
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.
The text was updated successfully, but these errors were encountered: