jupyterlab / jupyter-renderers

Renderers and renderer extensions for JupyterLab
BSD 3-Clause "New" or "Revised" License
490 stars 77 forks source link

[3.1.0rc2] `jupyterlab-fasta` fails to render: `t.on is not a function` #258

Closed lumberbot-app[bot] closed 2 years ago

lumberbot-app[bot] commented 3 years ago

Description

The jupyterlab-fasta extension does not work with 3.1.0rc2

Reproduce

Can be reproduced with this gist:

image

Expected behavior

The jupyterlab-fasta extensions should work out of the box with JupyterLab 3.1.

Context

Originally reported by @bollwyvl in https://github.com/jupyterlite/jupyterlite/issues/284


Originally opened as jupyterlab/jupyterlab#10675 by @jtpio, migration requested by @blink1073

fcollonval commented 3 years ago

@bollwyvl @jtpio

This seems to be msa / Firefox related: https://github.com/wilzbach/msa/issues/257

Xref: jupyterlite/jupyterlite#284

rvalieris commented 3 years ago

same error on chromium: 2021-08-07-142955_863x599_scrot

jupyterlab: 3.1.2 jupyterlab-fasta: 3.1.1 chromium: 92.0.4515.131

guignonv commented 3 years ago

Replace if(this.seqs=[],!t instanceof Array||"at"in t)) by if(this.seqs=[],!t instanceof Array||"on"in t). I guess there was a typo "at" --> "on".

jtpio commented 3 years ago

Thanks @guignonv! Would you like to open a PR?

zacharyrs commented 3 years ago

So the upstream library (https://github.com/wilzbach/msa) looks abandoned (last PR in 2018 and the NPM package was pushed 5 years ago).

The root issue isn't in that library, however, but is in https://github.com/wilzbach/stat.seqs, which also looks abandoned. Specifically, it's in https://github.com/wilzbach/stat.seqs/blob/master/lib/index.js#L52.

The issue can't easily be fixed here (unless you do a hacky find and replace at the end of the build stage). It can't be fixed upstream given they seem abandoned.

Thankfully, there's a fork with fixes - https://github.com/niaid/msa. The specific fix is https://github.com/niaid/msa/commit/e82512e8512e46d77e15ff04368596d9504b277a.

Unfortunately, this isn't published to NPM and is a bit of a pain to get working.

  1. Update the "msa" entry of package.json in the fasta-extension folder to point to "https://github.com/niaid/msa".
  2. Run the install steps, which fails due to the bio.io package not being built.
  3. Go into bio.io in node_modules and manually 'build' it by running babel -d lib src.
  4. Repeat the install step and you'll have a working FASTA viewer.

Hopefully, this helps a little...

jtpio commented 3 years ago

Thanks @zacharyrs for investigating this.

Thankfully, there's a fork with fixes - https://github.com/niaid/msa. The specific fix is niaid/msa@e82512e.

It would have been interesting to see if they planned to open a PR to merge the fixes upstream. Although as you mentioned it doesn't seem to be actively maintained unfortunately.

Another option would also be to maintain a fork of the msa library, for example in https://github.com/jupyterlab-contrib (and move the jupyterlab-renderers there too). But this adds quite a bit of maintenance cost.

zacharyrs commented 3 years ago

Personally I suspect we'd be best to move to their fork.

For reference, NIAID is the National Institute of Allergy and Infectious Diseases, so I suspect their fork will be well maintained.

I've contacted them to find out if they'll be publishing it to NPM, else I suppose we'll need a fork...

zacharyrs commented 2 years ago

Hey @jtpio! Unfortunately I don't thing NIAID will be publishing their fork to NPM. I think it might be worth forking it to https://github.com/jupyterlab-contrib and publishing that to NPM ourselves.

jtpio commented 2 years ago

Thanks @zacharyrs for following up here.

Would you like to help with this and releasing to npm? Probably we could then fork to https://github.com/jupyterlab-contrib and add you as maintainer of the repo?

The package could be published under the @jlab-contrib/msa name for example.

zacharyrs commented 2 years ago

Hey @jtpio, I'm keen to give it a go but I'll be honest that I'm not sure how much time I have to dedicate long-term. I can at least get the NPM builds working though so we can use it!

fcollonval commented 2 years ago

@zacharyrs I fork the NIAID msa repository in jupyterlab-contrib and you should have receive an invitation to become admin: https://github.com/jupyterlab-contrib/msa

fcollonval commented 2 years ago

Would you mind giving me your npm user id so I can invite you to the NPM organization?

zacharyrs commented 2 years ago

@zacharyrs I fork the NIAID msa repository in jupyterlab-contrib and you should have receive an invitation to become admin: https://github.com/jupyterlab-contrib/msa

Awesome! I've just accepted that.

Would you mind giving me your npm user id so I can invite you to the NPM organization?

Should be zacharyrs.

Thanks!

fcollonval commented 2 years ago

Thanks I invited you to the jlab-contrib npm organization. Let us know if you have trouble publishing the package.

jtpio commented 2 years ago

Released in https://pypi.org/project/jupyterlab-fasta/3.2.0