Codesandbox-client: Google fonts are loaded as <script> instead of <link>

Created on 4 Sep 2017  ·  14Comments  ·  Source: codesandbox/codesandbox-client


🐛 Bug 🔰 Beginner Friendly

All 14 comments

Ooh good one, logic for this resides here: https://github.com/CompuIves/codesandbox-client/blob/master/src/sandbox/external-resources.js.

Maybe we should either do a manual check, or do a preflight request to find out the Content Type?

It would be nice if the API could figure out the resource type when you add it (even return an error if it's neither JS, nor CSS), and return it. I don't know about the backwards compatibility, though.

I'm interested in working on this.

It seems like you could just use a HEAD request and then check the returned Content-Type. Example pseudocode:

function addResource(resource: string) {
  const match = resource.match(/\.([^.]*)$/);

  if (match && match[1] === 'css') {
    addCSS(resource);
  } else if (match && match[1] === 'js') {
    addJS(resource);
  } else {
    head(resource).then((response) => {
      // Check reponse
      if (contentType(response) === 'text/css') {
        addCSS(resource)
      } else {
        addJS(resource);
      }
    }).catch((e) => {
      // Should we log an error?
      addJS(resource)
    })
  }
}

Some questions:

  1. Does calling code expect addResource to be synchronous?
  2. Is there a standard way of doing resource fetching in this project?

That's really great @tansongyang! Your proposal sounds great.

Answering the questions:

  1. addResource can be asynchronous, everything in the sandbox itself is executed in an asynchronous manner
  2. For the sandbox we don't have a standard way of fetching, mostly to keep the bundle size low. I'd use window.fetch to fetch the resource.

I think we should log the error using console.error, mostly developers use CodeSandbox and maybe they can include it in the bug report when they occur this.

@CompuIves OK. One possible issue with fetch is that it isn't supported in IE11. Are you OK with codesandbox not supporting IE11?

@tansongyang No problem! We polyfill fetch, so it should also work on IE11.

Maybe also check for javascript Content-Type and error otherwise?

Sounds good, throwing an error is the best way. This way we can notify the
developer that an external resource isn't available.

On Tue, Sep 12, 2017, 10:05 lbogdan notifications@github.com wrote:

Maybe also check for javascript Content-Type and error otherwise?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/CompuIves/codesandbox-client/issues/174#issuecomment-328774642,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAj1CHgtDUrTbuQ0i-FR6LWs71uu4fdRks5shjsxgaJpZM4PLV8W
.

Sounds good. I'll post here when I have a PR.

Made PR. See the comments and commit messages for more details.

As a workaround, it looks like you can just add '.css' to the end of the url and it'll work: https://fonts.googleapis.com/css?family=Roboto:400,700.css

Same thing happens with Material icons: https://fonts.googleapis.com/icon?family=Material+Icons. And adding .css to the URL doesn't work.

If you add the weight, it works:
https://fonts.googleapis.com/icon?family=Material+Icons:400.css

Closing this as we can now load scripts and style-sheets from index.html.

Was this page helpful?
0 / 5 - 0 ratings