jupyter / notebook

Jupyter Interactive Notebook
https://jupyter-notebook.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
11.79k stars 5k forks source link

Avoiding preact #2508

Open infinity0 opened 7 years ago

infinity0 commented 7 years ago

Hi, I'm trying to update the Debian package to 5.0.0, it's on 4.2.3 at the moment. Looking through the sources and the npm packages, I don't think it is going to be humanly possible to package preact, preact-compat and proptypes for Debian, so I am looking to drop all functionality that depends on it from Jupyter notebook.

Fortunately, it seems that only these parts are affected:

notebook/static/notebook/js/shortcuteditor.js:    var render = preact.render;
notebook/static/notebook/js/shortcuteditor.js:    var createClass = preactCompat.createClass;
notebook/static/notebook/js/shortcuteditor.js:    var createElement = preactCompat.createElement;

Could you give me some tips on how to minimise the effect this will have on the Debian package? Thanks.

infinity0 commented 7 years ago

BTW, this seems wrong: https://github.com/jupyter/notebook/blob/master/notebook/static/notebook/js/actions.js#L67

        'toggle-rtl-layout': {
            help: 'Open a dialog to edit the command mode keyboard shortcuts',
[..]
        'edit-command-mode-keyboard-shortcuts': {
            help: 'Open a dialog to edit the command mode keyboard shortcuts',

What should the first one really say?

infinity0 commented 7 years ago

So, I think it would be enough if we just patch Jupyter notebook in Debian so that this bit in notebook.js:

    Notebook.prototype.show_shortcuts_editor = function() {
        new ShortcutEditor(this);
    };

instead shows a UI notice that says "sorry this feature is not available in Debian". Could you give me some tips on how to do this?

rgbkrk commented 7 years ago

I don't think it is going to be humanly possible to package preact, preact-compat and proptypes for Debian

Is the same true for React?

infinity0 commented 7 years ago

Yes - there's far too much of it, even more so than preact. It's a balance-of-time thing, if we tried to spend effort on these things, nothing else would get done in Debian. The easiest way here I think would just be to drop the shortcuteditor, assuming I was correct about nothing else needing preact/react.

rgbkrk commented 7 years ago

What would you plan on for phosphor and jupyterlab? We do still want to be able to use npm packages.

infinity0 commented 7 years ago

We can cope with particular npm packages, just not these ones.

Last time I looked at phosphor it had no dependencies and we've packaged 1.0.0/1.1.0 fine. I haven't looked at jupyterlab in detail - but for ipywidgets 6 only some type definitions were used so I just copied and pasted about 3 .ts files from jupyterlab/services rather than packaging it in its entirety.

ellisonbg commented 7 years ago

@infinity0 that may not be sustainable. While we are not using react/preact in JupyterLab right now, and we may keep it out of the core, the entire model of JupyterLab is to embrace npm packages and offer a rich extension system based on it.

infinity0 commented 7 years ago

Well, we'll have to see what that means in practise. My own interest is just packaging enough things to keep SageMath working. I'm not sure what JupyterLab is exactly, but I imagine most of it would be optional for SageMath's purposes.

Indeed, the npm ecosystem is full of practises that make things hard for package managers. If you could avoid packages that do these sorts of practises, that would be helpful for me and others. If not, then we'll perpetually have to find workarounds. For example, packages:

This isn't good for users either, at least ones that care about using verifiably-open source code. We have to follow these standards when packaging things for Debian, unlike the npm ecosystem which enforces none of these standards. Yes, not having standards makes certain aspects of development very easy for developers, but you should consider other participants of the software ecosystem as well, when writing software.

minrk commented 7 years ago

I'm not sure what JupyterLab is exactly,

JupyterLab is the planned replacement of the UI/javascript part of this repo. It will be the primary, official UI for Jupyter Notebooks.

I think it's a mistake for debian packages to unbundle npm dependencies like this. Each npm package that's part of a given application (e.g. jupyter-notebook, or any actually npm-based application) should include all of its own dependencies in the package, and not unbundle/link the npm dependencies as their own debian packages. That's not how npm packages work, and it's now how they should work when installed via other systems. While there may be some expansion of the build process for Debian's additional audit requirements, adding npm dependencies to a package should not result in requiring the creation of any additional debian packages.

infinity0 commented 7 years ago

The "bundling" approach is only suitable for large teams with enough developer resources to follow all of the transitive dependencies' development to pull in critical bugfixes. The vast majority of developer teams don't have the time nor resources to do this, so the final result is that nobody knows what code is actually being run, how out-of-date this is, and what sort of bugs it contains. This is very easy-to-use from a developer point-of-view, but is a complete abdication of one's responsibility to one's users. Debian and other distros unbundling dependencies, and ensuring that there is only one copy of any particular piece of software available in the whole distribution, helps to avoid this mess and is a very efficient way of using volunteer resources to provide these QA guarantees.

Every new version of a given package that is kept coinstallable with other versions (e.g. because they are bundled as dependencies of two different larger pieces of software), is effectively keeping a fork of that package, along with all the maintenance costs it brings. Ignoring these costs and pretending they don't exist, just piles up the technical debt and means users end up running unmaintained software.