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

Inline SVGs not rendering and break page #3179

Closed
oriooctopus opened this issue Jul 31, 2020 · 14 comments · Fixed by #3202
Closed

Inline SVGs not rendering and break page #3179

oriooctopus opened this issue Jul 31, 2020 · 14 comments · Fixed by #3202
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: starter Issues that are starter difficulty level, e.g. minimal tweaking with a clear test plan. good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin. help wanted Asking for outside help and/or contributions to this particular issue or PR. mlh Major League Hacking Fellowship

Comments

@oriooctopus
Copy link
Contributor

oriooctopus commented Jul 31, 2020

🐛 Bug Report

This may be user error, as I feel like this is core enough that it would have already been reported, so I apologize if I waste anyone's time.

Have you read the Contributing Guidelines on issues?

Yes

To Reproduce

  1. npx @docusaurus/init@next init my-website classic
  2. Add the following to the top of mdx.md:
import Mountain from '@site/static/img/undraw_docusaurus_mountain.svg';

A mountain:
<Mountain />
  1. Go to http://localhost:3000/docs/mdx The page should be broken and the following error should be in the developer panel:
    DOMException: Failed to execute 'createElement' on 'Document': The tag name provided ('/assetsundraw_docusaurus_mountain-b480421ae597bc5783ec62d605645b24.svg') is not a valid name.

I want to add that I also tried including the svg in an external component file with the same results

Your Environment

This was done with the most recent version of docusaurus

Reproducible Demo

I'm not including a link because the steps to reproduce from scratch are so quick. Hopefully that's ok!

@oriooctopus oriooctopus added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Jul 31, 2020
@teikjun
Copy link
Contributor

teikjun commented Jul 31, 2020

Hi, you can include svgs (and images) like this:

<img src='../static/img/undraw_docusaurus_mountain.svg' />

Or like this:

![img](../static/img/undraw_docusaurus_mountain.svg)

You can also choose to create a React component that renders the svg (in a .js file). Then, you can import it in your doc file and refer to it as <Mountain />. In this case, your doc file needs to end with .mdx, mdx allows you to use jsx together with your markdown.

@oriooctopus
Copy link
Contributor Author

oriooctopus commented Jul 31, 2020

Hi @teikjun thanks for your response. Unfortunately, even when the file is named mdx.mdx, the same error occurs. Regarding the other ways to include it, those are not inline, and I am trying to include inline svgs instead of external ones

@slorber
Copy link
Collaborator

slorber commented Jul 31, 2020

Hi,

import Mountain from '@site/static/img/undraw_docusaurus_mountain.svg';

A mountain:
<Mountain />

This is not std javascript to import a svg, this is Webpack specific, and there's no unique way to handle a svg file import:

  • we could get the SVG as a react component
  • we could get the SVG as source string markup
  • we could get a URL for downloading a svg source file
  • ...

This is not a bug to me, but a feature request, and Docusaurus user may want different behaviors according to their own usecases


In the meantime, you should be able to solve your problem by using "inline" Webpack loaders, directly in your import expression, and using a loader like: https://github.com/jhamlet/svg-react-loader

Add the dependency, and then:

# Some markdown title 

import Icon from 'svg-react-loader?name=Mountain!@site/static/img/undraw_docusaurus_mountain.svg';

A mountain:
<Mountain />

I'm closing, but open to discuss a feature request for this.

@slorber slorber closed this as completed Jul 31, 2020
@oriooctopus
Copy link
Contributor Author

oriooctopus commented Jul 31, 2020

Hi @slorber the thing is that in the docs https://v2.docusaurus.io/docs/static-assets#jsx-example states that inline svgs are an option.
I should also add I tried putting the svg import in an external component and then importing that into the mdx file, with the same error

@slorber
Copy link
Collaborator

slorber commented Jul 31, 2020

Hmmmm, I didn't know we had that!

Need to check this next week, but I can confirm I see this webpack loader:

        {
          test: /\.svg$/,
          use: '@svgr/webpack?-prettier-svgo,+titleProp,+ref![path]',
        },

@slorber slorber reopened this Jul 31, 2020
@oriooctopus
Copy link
Contributor Author

Awesome! Thank you!

@slorber slorber added difficulty: starter Issues that are starter difficulty level, e.g. minimal tweaking with a clear test plan. good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin. help wanted Asking for outside help and/or contributions to this particular issue or PR. mlh Major League Hacking Fellowship and removed status: needs triage This issue has not been triaged by maintainers labels Aug 3, 2020
@anshulrgoyal
Copy link
Contributor

I will take this.

@anshulrgoyal
Copy link
Contributor

I checked the code and the problem is that there are conflicting webpack-loaders for .svg files. Both url-loader and @svgr/webpack try to load svg. If we disable url-loader for .svg it works fine. @slorber what should we do?

@anshulrgoyal
Copy link
Contributor

We can use something like this.

{
  test: /\.svg$/,
  use: ['@svgr/webpack', 'url-loader'],
}

then it won't break existing websites.

import starUrl, { ReactComponent as Star } from './star.svg'
 
const App = () => (
  <div>
    <img src={starUrl} alt="star" />
    <Star />
  </div>
)

@slorber
Copy link
Collaborator

slorber commented Aug 3, 2020

I think in the image loader of webpack:

    images: (): RuleSetRule => {
      return {
        use: [loaders.url({folder: 'images'})],
        test: /\.(ico|svg|jpg|jpeg|png|gif|webp)(\?.*)?$/,
      };
    },

we can just remove the svg from here, as:

  • Using imports/requires, we expect to get a React comp
  • Using md images, we already force the file-loader

Can you find a way to dogfood this feature somewhere by rendering an svg as both a md image + a mdx react component? That would help us not break this feature again in the future.

(opened also an issue to improve the assets doc as it's on 2 pages #3198)

@anshulrgoyal
Copy link
Contributor

But if we remove svg from url-loader it might break pages for some users in the next release.

@slorber
Copy link
Collaborator

slorber commented Aug 3, 2020

This is a new feature, I don't think it has been used by users in this small amount of time, and also, this feature is actually in contraction with our existing svg to React conversion in the first place, so I'd rather revert to the correct behavior we had before alpha 59, even if it's a breaking change.

@slorber slorber linked a pull request Aug 5, 2020 that will close this issue
@oriooctopus
Copy link
Contributor Author

Thank you @slorber !

@slorber
Copy link
Collaborator

slorber commented Aug 5, 2020

thank @anshulrgoyal :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: starter Issues that are starter difficulty level, e.g. minimal tweaking with a clear test plan. good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin. help wanted Asking for outside help and/or contributions to this particular issue or PR. mlh Major League Hacking Fellowship
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants