Google fonts are loaded as <script> instead of <link>

[lbogdan]

[CompuIves]

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?

[lbogdan]

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.

[tansongyang]

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?

[CompuIves]

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.

[tansongyang]

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

[CompuIves]

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

[lbogdan]

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

[CompuIves]

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 ***@***.***> 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 <#174 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAj1CHgtDUrTbuQ0i-FR6LWs71uu4fdRks5shjsxgaJpZM4PLV8W> .

[tansongyang]

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

[tansongyang]

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

[eschafer]

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

[mrchief]

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

[eschafer]

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

[lbogdan]

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