Skip pending request if new content is set #24
Conversation
juicy-html.html
Outdated
| */ | ||
| JuicyHTMLPrototype.skip = function() { | ||
| if(this.pending){ | ||
| this.pending.onload = null; |
There was a problem hiding this comment.
Did you consider this.pending.abort();?
There was a problem hiding this comment.
I did. But since we cannot even mimic such abort with HTML imports, I'd rather keep both element consistent.
juicy-html.html
Outdated
| * Skips/disregards pending request if any | ||
| * @return {XMLHttpRequest} skipped XHR request, in fact just `this.pending` | ||
| */ | ||
| JuicyHTMLPrototype.skip = function() { |
There was a problem hiding this comment.
Since _loadExternalFile is "private", maybe this method should be called _skip (with an underscore)? Or maybe a fuller name _abortLoadingExternalFile().
There was a problem hiding this comment.
I think it's worth keeping it private, as stracounter-include may use it.
There was a problem hiding this comment.
You mean keeping it public?
Anyway I opt for more informative method name, like abortLoadingExternalFile. And I suggest to not return anything, since nowhere you are using the return value (YAGNI).
There was a problem hiding this comment.
Yes "keeping it public" sorry.
I don' like abort... as we are not aborting the loading of external file, what about skipStampingPendingFile?
Personally, I don't like YAGNI approach, especially in the world where we don't have any requirements and all we do is foreseeing potential applications.
Also from functional, mathematical point of view I prefer functions that returns something. They are easier to test and comprehend.
However if you are really on it, I may remove this return
There was a problem hiding this comment.
You prefer them, because they are harder to test and comprehense? For this reason I don't prefer them, sorry :P
There was a problem hiding this comment.
skipStampingPendingFile is also ok for me.
The fact that I has confused about what does this method do proves that just calling it skip is not enough. "Skip what?" is the question that I want to be answered by the method name.
There was a problem hiding this comment.
You prefer them, because they are harder to test and comprehense? For this reason I don't prefer them, sorry :P
Sorry, I meant
Also from functional, mathematical point of view I prefer functions that returns something. They are easier to test and comprehend.
There was a problem hiding this comment.
I don't think they are easier to test, because you have one more thing to test - the return value.
And none of the existing tests checks it.
tomalec
force-pushed
the
skip-when-url-changed
branch
from
f2202cd to
de2d961
Compare
|
@warpech What about now? |
as discussed at Juicy/juicy-html#24
Starcounter/starcounter-include/issues/25