asset-rack icon indicating copy to clipboard operation
asset-rack copied to clipboard

[feature] allow empty contents

Open binarykitchen opened this issue 12 years ago • 13 comments

hello guys

this might sound weird, but i have a special situation here where i'd like to set null for contents. at the moment this code works fine:

        // create special kind of settings for a json file that is hashed by its contents
        var settingsAsset = new rack.Asset({
            url:      '/j/app.settings.json?' + app.settings.hash,
            contents: ''
        });

        // overwrite respond: i need the req variable to build contents
        // (because my settings depend on user-agent in req) 
        settingsAsset.respond = function(req, res) {
            var contents = app.settings.filter(req);
            res.json(contents);
        };

you see, this is very special. i'm generating the real contents during the response() method. if i leave out contents: '' then i get a 404.

i think my above code isn't elegant but i see no other workaround. i want to be able to build the contents with the help of contents inside the response variable res.

any ideas?

binarykitchen avatar Apr 13 '13 04:04 binarykitchen

Why use asset-rack for this? Why not just make use app.get? If the contents are dynamic its not really static is it? So asset-rack wouldn't actually do anything.

noc7c9 avatar Apr 13 '13 05:04 noc7c9

hmmm, with app.get? no idea how ... can you show me?

binarykitchen avatar Apr 13 '13 05:04 binarykitchen

app is the express instance.

app.get('/j/app.settings.json', function(req, res) {
    var contents = app.settings.filter(req);
    res.json(contents);
});

This is just the express api. Any reason it won't work in your case?

PS: What's the purpose of using app.settings.hash?

noc7c9 avatar Apr 13 '13 06:04 noc7c9

ah, thanks. with a little modification it worked:

        app.get('/j/app.settings-' + app.settings.hash + '.json', function(req, res) {
            res.set({'cache-control': 'public, max-age=' + maxAge});
            res.json(app.settings.filter(req));
        });

purpose of app.settings.hash is to force client to reload it whenever settings have changed. somewhere else in the code i have this:

settings.hash = crypto.createHash('md5').update(JSON.stringify(settings)).digest('hex');

binarykitchen avatar Apr 13 '13 06:04 binarykitchen

Oh for cache busting? I didn't really think cache busting would apply since you're generating it dynamically. But that makes sense.

noc7c9 avatar Apr 13 '13 07:04 noc7c9

yep, it makes sense like that. exactly what i need.

maybe you want to add that scenario to asset-rack too? no idea, maybe a new class or allow custom contents by response?

binarykitchen avatar Apr 13 '13 07:04 binarykitchen

I think the problem is that you can't add assets dynamically.

Maybe this would be a good feature to have for this use case.

techpines avatar Apr 15 '13 01:04 techpines

Yes, would be very nice. And probably be linked with https://github.com/techpines/asset-rack/issues/43

binarykitchen avatar Apr 15 '13 01:04 binarykitchen

@noc7c9 Worked on a low level syntax for adding assets.

Maybe, a syntax like this would be beneficial:

assets.add(new rack.Asset({
    url: '/j/app.settings.json',
    contents: ''
}));

techpines avatar Apr 15 '13 13:04 techpines

@binarykitchen I have another idea, please try this out.

var settingsAsset = new rack.Asset({
    url: '/j/app.settings.json',
    contents: JSON.stringify(app.settings) // <-- to allow the Asset to handle cache-busting
});

originalRespond = settingsAsset.respond;
settingsAsset.respond = function(req, res) {
    this.contents = app.settings.filter(req); // <-- dynamically set the content
    this.emit('created');
    originalRespond.apply(this, arguments);
};

Note: I haven't tested this, so I have no idea if this will even run :P.

The point here is that you're generating user-specific content from the a static source, so the setting the contents to the static source allows asset-rack to provide the cache-busting. And instead of overriding respond you instead wrap it to provide the necessary functionality.

This code would look so much better if I could use super o.o

@techpines What @binarykitchen is trying to do here is serve _dynamic_ content from an unchanging url which is completely unsupported by asset-rack. And I'm of the opinion that this is outside what asset-rack should handle since this isn't static content at all. Also the above fix is similar to how I imagine file watching will have to be implemented, so this could be a good test run ;D

noc7c9 avatar Apr 15 '13 14:04 noc7c9

@noc7c9 thanks for your idea - i tried it and i'm sorry it didn't work. in the footer i added != assets.tag('/j/app.settings.json') but firebug shows that it's never requested. of course i added settingsAsset to the assets for app.use(new rack.Rack(assets));

probably a little bug somewhere?

binarykitchen avatar Apr 16 '13 01:04 binarykitchen

haa... It didn't did it? Try omitting the this.emit('created'); call, my testing showed some pretty funky stuff happening when you call that. This won't support gzip anymore though, which is a bummer :(

noc7c9 avatar Apr 16 '13 21:04 noc7c9

No gzip? No thanks, sorry.

binarykitchen avatar Apr 17 '13 01:04 binarykitchen