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

Timeout issue with built version of code #27

Closed
Integralist opened this issue Dec 19, 2012 · 8 comments
Closed

Timeout issue with built version of code #27

Integralist opened this issue Dec 19, 2012 · 8 comments

Comments

@Integralist
Copy link

Hi,

I'm having a problem getting this code to work when built via the Node/r.js build script.

Here is the example/demo project I'm working with: https://github.com/Integralist/RequireJS-CSS-Loader

The pre-build files /www/work fine, but the built version /www-built/ doesn't. Instead it executes the callback (as if the styles have been loaded) but it doesn't render the styles. Then after a few seconds the RequireJS timeout error appears in the console.

This happens regardless of whether the styles are inlined or kept external via the separateCSS option in the build script.

Would it be possible for you to take a quick scan through the example repo and let me know what the problem might be.

Thanks.

@guybedford
Copy link
Owner

I had a quick look and it seems the issue is solved by changing your map config to remove the ./ and instead write:

    map: {
        '*': {
            'css': 'plugins/css'
        }
    }

The ./ notation is only needed for relative paths within other modules.

@Integralist
Copy link
Author

@guybedford legend! that worked thanks :-)

One extra query is now about the separateCSS option which doesn't appear to work for me? If you check the updated repo you'll see an extra folder there called www-built-separate.

The build script was updated to include the extra option, like so...

({
  appDir: 'www',
  dir: 'www-built-uglified',
  baseUrl: '.',
  fileExclusionRegExp: /(.git)$/,
  separateCSS: true,
  //optimize: 'none',
  map: {
    '*': {
      'css': 'plugins/css'
    }
  },
  modules: [
    {
      name: 'main'
      /*
      name: 'main',
      separateCSS: true,
      create: true
      */
    }
  ]
})

// node r.js -o build.js

...but when viewing the page in the browser the styles weren't applied (no errors of any kind, just the styles weren't applied?)

I had assumed that this option would have meant that RequireJS would asynchronously load the style sheet at run time, but when I check the 'net' panel in Firebug I see after main.js there are no further requests.

Any idea? Hopefully it is an easy a fix as the previous query I had.

Thanks.

@guybedford
Copy link
Owner

Great, glad it's working. SeparateCSS creates another file called main.css, containing all the css for the build layer, which can be manually included with a link tag in the page. It's mainly created for those who don't like the idea of loading CSS from a script tag, although I have yet to meet such a person.

For runtime loading, simply load the layer itself asynchronously at runtime.

@Integralist
Copy link
Author

hmm, ok. I can see it generates a main.css which looks pointless in this example because I only have one rule in my stylesheet, but I guess if I was loading lots of stylesheets then this would end up being a single concatenated version of potentially multiple CSS files being used.

I'm not sure why any one would want to load the style sheet via the HTML <link> tag because effectively they're back to square one: if no js enabled then they've loaded a lot of CSS for no reason?

But my concern now is: if i wanted to load this single generated stylesheet at run time rather than having it inlined with the main script - I would need to insert the stylesheet into the page and potentially hit the issues most people have when checking when the style sheet is loaded and thus have to work around that issue.

I understand that's obviously why you've chosen the route of inlining the stylesheet content inside the script itself but I just wanted to make sure I had properly understood the intention/purpose of this RequireJS extension you've written (I think I've misunderstood it to be like the Curl.js AMD library which hacks around the lack of onload support, but in fact you're side stepping that problem by inlining the CSS with the script).

Thanks again by the way for your feedback, it's been very helpful :-)

@guybedford
Copy link
Owner

The side stepping is caused by allowing the CSS to be a dependency of the rendering code. In this way DOM injection always happens after style injection, with no need to check loading.

It's best not to use separateCSS in your case, but rather create two built files, something like this:

modules: [
    {
      name: 'main'
      exclude: ['widget']
    },
    {
      name: 'widget'
    }
  ]

And then have widget.js:

define(['css!style/style'], function (component) {
    // CSS Builder Object
    console.dir(component);

    // Now we know the Style Sheet is loaded we can insert our widget
    var widget = document.createElement('h1');
        widget.innerHTML = 'test';
    document.body.appendChild(widget);
});

and finally change the require in main.js to:
require(['widget']);

@Integralist
Copy link
Author

OK I'll give that a go in a little bit and get back to you. Thanks again.

@Integralist
Copy link
Author

I tried that and it worked how I expected it to: i.e. the main.js is now the bootstrapper script (which is tiny for the initial page load) and it asynchronously loads the widget module separately.

So really the decision that I'd need to make is which route to go down...

A.) include the styles inline as part of the main.js script

or

B.) load the widget module asynchronously (which still includes the styles inline but means the bootstrap is as small as possible).

The trade off being the overall size against an extra HTTP request.

Thanks Guy! Really appreciate the help today.

@guybedford
Copy link
Owner

@Integralist typically it's a good idea to include as much as possible in the core bootstrap, until it starts getting to a certain size limit. I would expect a good cutoff would be around 100 - 200KB, but this should probably be tested. For multi-page apps, the core-built widgets would be those included for most pages, while separate widget layers allow collections of functionality specific to certain app sections.

The other thing worth considering is changing the widget from inserting itself into the dom, to having a 'render' function. In this way, the widget is decoupled from where it renders to. Then a separate 'renderer' can run the rendering of widgets. I've implemented a basic rendering specification as part of ZestJS. If you're interested you can check out the alpha version here - https://github.com/zestjs/zestjs.org, which contains docs explaining the process better.

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

No branches or pull requests

2 participants