Skip to content
This repository has been archived by the owner on Apr 30, 2020. It is now read-only.

Icons are not rendered correctly #194

Closed
jebovic opened this issue Sep 5, 2019 · 14 comments · Fixed by #195
Closed

Icons are not rendered correctly #194

jebovic opened this issue Sep 5, 2019 · 14 comments · Fixed by #195

Comments

@jebovic
Copy link

jebovic commented Sep 5, 2019

Hi,
I'm trying to install the extension on the last version of jupyterlab (v1.1.1).
As you can see on the screenshot below, icons are not rendered anymore.

image

After investigation, it seems that the upgrade of babel from version 5 to 6 is responsible of this issue. It's certainly updated in the dependency tree of other packages and extensions in the Jupyter environment.

The require function behavior has changed, and the call to the "require" function (here: https://github.com/jupyterlab/jupyterlab-celltags/blob/master/src/components/tag.tsx#L117) now returns an object, and not a string as expected.

From web browser :

image

From the code, this call returns an object :
src={require('../../static/add_blue.svg')}

The fix which returns what's expected for a src attribute :
src={require('../../static/add_blue.svg').default}

Another solution would be to use this plugin : https://www.npmjs.com/package/babel-plugin-add-module-exports

Regards,
Jérémy

@choldgraf
Copy link
Contributor

I've got the same issue!

@timkpaine
Copy link
Member

its widespread :-(

@choldgraf
Copy link
Contributor

@timkpaine ah that's a bummer :-/ I wonder if this is a good opportunity for someone to write a short Discourse post on what the problem is so that other extension developers can discover the fix? I assume it's the same problem across all of them (and I'm guessing it's some kind of dependency version bump issue?)

@blink1073
Copy link
Member

@telamonian, I think this is related to jupyterlab/jupyterlab#6034, mind taking a look? It says "Could not load the image" when I hover over the image src below:

image

@telamonian
Copy link
Member

telamonian commented Sep 8, 2019

@blink1073 Celltags uses a different means of inlining SVGs than what I worked on. I'm not super familiar with the require function or with babel, so I can't comment on what broke. This relied on the fact that, by convention, require-ing an .svg would load the whole thing as a raw string. At some point the convention changed (I think ES5 vs ES6), so now you need the

require('<path-to-svg>/foo.svg').default

form that @jebovic mentioned in their original post.

As an alternative, the problem could potentially be fixed using some of the machinery that jupyterlab/jupyterlab#6034 adds. The best way to expose said machinery to external extensions hasn't really been worked out, but this could be a chance to do so. I'll look into it.

@choldgraf
Copy link
Contributor

choldgraf commented Sep 21, 2019

It seemed from the comments in this thread that all we had to do was add default to the image calls, so I did that in

#195

No idea if that's right or not, because I'm not a javascript developer, but if that's all the fix needs can we get it merged? This extension's UI is broken right now...

@DancingQuanta
Copy link

I have this issue too!

@telamonian
Copy link
Member

I figured out exactly why this bug happened when it happened. In Jupyterlab v1.1, in the build system, the Webpack loader for .svg files changed from url-loader to raw-loader. The loader determines the details of the implementation of the require function. For the url-loader, this statement:

require('<path-to-svg>/foo.svg')

used to return a string, but for the raw-loader it returns a module object. The .default property of that module object is a string with the raw contents of the imported .svg. That's why you now have to add the .default to get the import to work correctly.

@rftw
Copy link

rftw commented Oct 5, 2019

I figured out exactly why this bug happened when it happened. In Jupyterlab v1.1, in the build system, the Webpack loader for .svg files changed from url-loader to raw-loader. The loader determines the details of the implementation of the require function. For the url-loader, this statement:

require('<path-to-svg>/foo.svg')

used to return a string, but for the raw-loader it returns a module object. The .default property of that module object is a string with the raw contents of the imported .svg. That's why you now have to add the .default to get the import to work correctly.

How can I install the new version of cell tags? I tried installing jupyterlab and jupyterlab/celltags but the icons are still not rendering properly

@telamonian
Copy link
Member

telamonian commented Oct 9, 2019

@rftw A new release (v0.2.0) of @jupyterlab/celltags has now been published that includes the fixes for this issue. If you uninstall/reinstall celltags, it should now be fixed for you

@choldgraf
Copy link
Contributor

It worked for me, hooray!

@rftw
Copy link

rftw commented Oct 9, 2019

Yup! working now. Thank you for fixing it

@telamonian
Copy link
Member

Glad to hear it. One nice side effect of prepping a new release was that I also cleared out this repo's PR backlog!

@Zsailer
Copy link
Member

Zsailer commented Oct 9, 2019

This is awesome, thanks @telamonian!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants