quack quack: only check for properties that would actually be used?
jonathanong opened this issue · 9 comments
https://github.com/jshttp/etag/blob/master/index.js#L87
thinking of just passing an object like { mtime, size }. what do you think?
only check for properties that would actually be used?
This is what we do :) We want those other properties so we can start to use them without a major version bump. It shouldn't be too hard to just pass bogus data, though? {mtime: mtime, atime: mtime, ctime: mtime, ino: 0, size: size}
ohhhh i didn't know you were going to actually use them. i'm not sure how using atime would make sense for etags though...
Yea, I didn't want to think about it too much at the time, only wanted to choose some set that didn't pidgin-hole us :) In fact, the only difference between our stat ETags and Apache's is that Apache includes inode, though it's not great for multi-server replication.
so... can we remove atime and ino and call it a day then? :D
Yea, I can do that for you :) For my curiosity, what are you trying to do here, in the end?
building a http2 static server that automatically pushes dependencies. i want to enable plugins like path => stats, but since many times you won't ever save it to disk, there won't be a real stat obj, so i want to allow makeshift stats objects that uses this module if they don't set a stats.etag themself.
just trying it though - not sure if i'm going to stick with stats objects, just trying to avoid abstractions.
but since many times you won't ever save it to disk
Well, why not make an ETag off the content, then? I only ask because if the content only exists in memory, theoretically you can then simply hash the contents into an ETag. Of course, you should also take into consideration that an ETag doesn't add any value if the client cannot check back for that ETag at a later time (or to another server in the server farm).
¯\_(ツ)_/¯ it's not fully thought out. just using stats right now and was wondering, "what if people made their own stuff?" also it might not always exist in memory - could point to an arbitrary location in the file system.
I know. I'm also just throwing around thoughts :) I'll update it to remove the restriction, probably tomorrow after Express 4.12 finally get released :O