-
Notifications
You must be signed in to change notification settings - Fork 2k
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
"unless" breaks when "each" value equals "null" #1319
Comments
It seems that if/else have the same bug. Updated test case: https://jsfiddle.net/r33cjjb2/4/ |
I tried to reproduce this in a simpler jsfiddle https://jsfiddle.net/r33cjjb2/6/. What you mean is, that the first line of output of my jsfiddle should have |
Hey nknapp, Sorry my example wasn't as simple as yours. It was a stripped down version of my actual use case, But yes that is correct, except that actually it's a bit worse than that. Yours only captures part of the issue. I updated yours again: Note that the first (relevant) line (after the 'unless' header) now reads as: (The object, as you will see if you catch it in a helper or something as I did for debugging my real project, contains no members.) It should match the 'no unless' case (I think), which comes out correctly as: As it is, all context values:
are containing unexpected values that are inconsistent with all other cases. If the unless is not present, you would get the correct result (as the updated example, or my original shows). In every other case they match. Also not included in this simpler version is the fact that {{else}} cases are also broken (if memory serves). I also have not tested {{#if}}{{else}}{{/if}}, but looking a bit at the handlebars source code (which I did on my phone on the train the day i submitted this), suggests it probably also has the same issue. Thanks for your time. -Jeremy |
Correction: I forgot that I DID test if/else case, the fiddle for it I had already posted above your first reply. |
Also I don't know if it is relevant or not, but my original one demonstrated that this was true for iterating over either objects or arrays. Your example only shows arrays. Perhaps this is enough test coverage for the problem, I don't know the internal implementation really so I error-ed on the side of caution and inclusiveness. Sorry if it was too much. |
My example did not cover all aspects, that's right. I just wanted to check if I understood the problem correctly. No problem. I have to dig into the code as well. It seems like the "unless"-helper just calls the if-helper with flipped blocks. |
One thing I noticed is that for the |
Fixes #1319 Original behaviour: - When a block-helper was called on a null-context, an empty object was used as context instead. (#1093) - The runtime verifies that whether the current context equals the last context and adds the current context to the stack, if it is not. This is done, so that inside a block-helper, the ".." path can be used to go back to the parent element. - If the helper is called on a "null" element, the context was added, even though it shouldn't be, because the "null != {}" Fix: - The commit replaces "null" by the identifiable "container.nullContext" instead of "{}". "nullContext" is a sealed empty object. - An additional check in the runtime verifies that the context is only added to the stack, if it is not the nullContext.
Fixes #1319 Original behaviour: - When a block-helper was called on a null-context, an empty object was used as context instead. (#1093) - The runtime verifies that whether the current context equals the last context and adds the current context to the stack, if it is not. This is done, so that inside a block-helper, the ".." path can be used to go back to the parent element. - If the helper is called on a "null" element, the context was added, even though it shouldn't be, because the "null != {}" Fix: - The commit replaces "null" by the identifiable "container.nullContext" instead of "{}". "nullContext" is a sealed empty object. - An additional check in the runtime verifies that the context is only added to the stack, if it is not the nullContext.
Fixes #1319 Original behaviour: - When a block-helper was called on a null-context, an empty object was used as context instead. (#1093) - The runtime verifies that whether the current context equals the last context and adds the current context to the stack, if it is not. This is done, so that inside a block-helper, the ".." path can be used to go back to the parent element. - If the helper is called on a "null" element, the context was added, even though it shouldn't be, because the "null != {}" Fix: - The commit replaces "null" by the identifiable "container.nullContext" instead of "{}". "nullContext" is a sealed empty object. - An additional check in the runtime verifies that the context is only added to the stack, if it is not the nullContext.
Thanks for your attention on the matter. Do you know how long it will take for the fix to turn up in the npm and bower packages? Do I need to wait for a version number increase? If so which one should I expect it will turn up in? |
Fixes #1319 Original behaviour: - When a block-helper was called on a null-context, an empty object was used as context instead. (#1093) - The runtime verifies that whether the current context equals the last context and adds the current context to the stack, if it is not. This is done, so that inside a block-helper, the ".." path can be used to go back to the parent element. - If the helper is called on a "null" element, the context was added, even though it shouldn't be, because the "null != {}" Fix: - The commit replaces "null" by the identifiable "container.nullContext" instead of "{}". "nullContext" is a sealed empty object. - An additional check in the runtime verifies that the context is only added to the stack, if it is not the nullContext. Backwards compatibility within 4.0.x-versions: - This commit changes the compiler and compiled templates would not work with runtime-versions 4.0.0 - 4.0.6, because of the "nullContext" property. That's way, the compiled code reads "(container.nullContext || {})" so that the behavior will degrade gracefully with older runtime versions: Everything else will work fine, but GH-1319 will still be broken, if you use a newer compiler with a pre 4.0.7 runtime.
I have made a slight change to my commit. In the previous version, templates compiled by a newer compiler would not work with the runtimes that a current publish (4.0.0 - 4.0.6). The latest change adds some compatibility code, which does not solve this bug (if you are using a runtime < 4.0.7) but at least it does not break anything else. This resolves my concerns that this might be a breaking change and since noone has felt obligated for a code-review I will just merge the changes into the 4.x and the master-branch. However, I cannot publish anything to the repositories and I don't know when anybody has the time (see #1312 ). It's not in my power to do that. |
I can see that I have made a typo in the commit message. It should read
What I wanted to write was: I had a solution ready, but it would have been breaking. So I added an additional check to the compiler code that would allow the templates to work with older runtimes. In order to get the bugfix however, you'll need a new runtime and compiler. Did you encounter a problem were runtime and compiled template were incompatible? |
@nknapp : Sorry it has taken so long to get back about this, I just had a better chance to dig into what is happening, and it is not related to this change. I will be opening a different issue/commenting on the change that introduced the issue I'm seeing, once I can get a code example that can be publicly shared. I will be deleting the comment that I made earlier in this thread in error. |
I had been facing the same issue while checking conditional statements in the DOM section to hide/show the content. So, I added this script in the backend as Handlebars Helpers. Handlebars.registerHelper('iff', function(a, operator, b, opts) {
}); Uses example {{#iff data.value "===" 45 }} |
still have the issue in v4.7.6, it should be reopened. |
When using "each" over either an object or an array and a null value is encountered, "unless" will destroy the context, replacing the null value with an empty object, and dropping parent access all together.
Here is a reproduction test case: https://jsfiddle.net/r33cjjb2/3/
To aid seeing the issue, the failing case renders the following inline: !!!FAIL!!!FAIL!!!FAIL!!!FAIL!!!FAIL!!!FAIL!!!FAIL!!!FAIL!!!FAIL!!!FAIL!!
Looking forward to seeing this fixed. It is a real pain in my @$$ atm. Otherwise thank you for a magnificent tool.
The text was updated successfully, but these errors were encountered: