-
-
Notifications
You must be signed in to change notification settings - Fork 4.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
[Bug] Ember's template compiler on 3.25 complains about ~ in a comment #19392
Comments
I was also seeing these errors after upgrading to 3.25.1 but it stoppped after i deleted node_modules and installed it again |
Hmm, this is odd indeed. It definitely sounds like a bug, but I'm not sure exactly what is going on. |
I could definitely use a reliable reproduction if someone has the time to try and isolate one... |
@rwjblue - here. Clone, P.S. There are obviously some caches at play because the second time I run This is the HBS that used to cause the warnings: {{! comment ~}}
<div></div>
<div>
{{~!~}}
<span></span>
</div> |
Ya, we cache the compilation result into system temp. You can disable that behavior (likely causing an error on every rebuild) with |
Well, in any case, that's concerning you mostly because you're fixing it. 😃 You managed to reproduce it, right? |
FWIW we're seeing similar issues:
interestingly we are only seeing it with |
@Turbo87 - What is the template snippet that causes that output? |
I can't tell because the snippet is not mentioned in the logs, but it appears to be happening for almost all of our templates, judging by the amount of these log lines. |
I'm slightly confused. Are these actual errors or is it console output during template compilation? If it is just console output (still horrible, don't get me wrong) does the application work properly (e.g. pass tests and what not)? |
just console output during the build steps.
yes, everything appears to be working as intended. or at least there is nothing visibly wrong and the tests are passing fine. since I don't know what is causing this message I can't tell if we're shipping any additional debug code or something like that, but our asset size tracking has not shown any significant increases from updating to 3.25, so that also seems unlikely. |
@rwjblue if you need a repro, the crates.io codebase is currently showing these warnings too :) |
OK, awesome. That is helpful, thank you! |
@boris-petrov which version of Ember are you using? We are on ember-source version |
@nelstrom - I am on the latest - 3.27.5. Not sure if that was what resolved the issue though. But the fact is that I haven't seen these warnings in a while now. |
@boris-petrov ok, that gives me some motivation to upgrade. |
I started getting these errors again after updating |
I'm seeing similar logs when updating
The I guess the issue (in my case) has always been there, but just wasn't logged. Any directions on how I can solve this would be greatly appreciated! |
I'm noticing a few not sure if it comes from there but my application and its addons (mono repo with yarn packages) are dependant on ember-cli-htmlbars@^5.7.2 |
I hit this in a repo that uses |
I'm also seeing it in an app with ember-css-modules after upgrading to 4.2. I found this in a project search, maybe someone comitted this by mistake (or is this generated?):
This looks different to the following similar code, which I guess is to do with different javascript ecosystems/modules/packagers. |
…bo87 Update dependency ember-css-modules to v2.0.1 [](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ember-css-modules](https://togithub.com/salsify/ember-css-modules) | [`2.0.0` -> `2.0.1`](https://renovatebot.com/diffs/npm/ember-css-modules/2.0.0/2.0.1) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>salsify/ember-css-modules</summary> ### [`v2.0.1`](https://togithub.com/salsify/ember-css-modules/blob/HEAD/CHANGELOG.md#​201-March-23-2022) [Compare Source](https://togithub.com/salsify/ember-css-modules/compare/v2.0.0...v2.0.1) ##### Fixed - Avoid triggering (ember.js#​19392)\[emberjs/ember.js#19392] when we produce synthetic class `AttrNode`s. </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/rust-lang/crates.io).
Is this an issue with glimmer.js? If yes, will it be fixed in 2.0? |
its a bug in the sense that it outputs a meaningless warning, its not actually an issue beyond that |
This is also impacting consumers of the I would argue this is well worth fixing (at a minimum: don't output the warning if the node is synthetic, if possible). To developers, this looks like there is a syntax error somewhere. Any warnings that appear during the course of migration are particularly distracting, because we are on the lookout for new problems. One could require plugins to set the attribute's
and key off that . . . ? |
PR for fixing the PR for fixing the remaining cases: https://github.com/glimmerjs/glimmer-vm/pull/1431/files |
@navels, thanks for the PRs! any plans to make checks pass? @NullVoxPopuli? |
We're waiting on some glimmer vm refactoring work atm, at which point, vm fixes very likely won't be back portable, depending on complexity of the change. We'll see what happens once the long overdue maintenance is done. (The linked fix above looks fairly isolated and straight forward tho) Stay tuned! |
Thanks, I will stay tuned. Happy to retest and update the PRs if they are still warranted. |
Any updates on this? Checking the code of https://github.com/glimmerjs/glimmer-vm/blob/master/packages/%40glimmer/syntax/lib/v1/public-builders.ts , it is not possible (or not easy) to set a useful |
Any updates on the refactoring that was blocking this fix? |
Someone probably needs to do the work in |
e.g. `unexpectedly found "! no whitespace ~" when slicing source, but expected " no whitespace "` See: emberjs/ember.js#19392
e.g. `unexpectedly found "! no whitespace ~" when slicing source, but expected " no whitespace "` See: emberjs/ember.js#19392 Co-authored-by: David Taylor <david@taylorhq.com>
This is a stopgap until emberjs/ember.js#19392 is fixed.
Re-apply #1430 with a more robust implementation and tests Also turns the warning into a `localAssert()` so it gets stripped before reaching downstream consumers. This relates to #1431 in that this removes the warning, so to the extent that #1431 was motivated by sliencing the warning in Ember builds, it avoids the issue. But I haven't investigated the root cause of that issue and whether it is indicative of an actual bug somewhere deeper. Fixes emberjs/ember.js#19392 Co-authored-by: Lee Nave <lee.nave@ovation.io>
Re-apply #1430 with a more robust implementation and tests Also turns the warning into a `localAssert()` so it gets stripped before reaching downstream consumers. This relates to #1431 in that this removes the warning, so to the extent that #1431 was motivated by sliencing the warning in Ember builds, it avoids the issue. But I haven't investigated the root cause of that issue and whether it is indicative of an actual bug somewhere deeper. Fixes emberjs/ember.js#19392 Co-authored-by: Lee Nave <lee.nave@ovation.io>
Running
ember serve
withember-source
3.25 leads to:These correspond to these two lines of HBS (they are in different files, I've just put them here for conciseness):
This didn't happen before. Not sure if this is a bug in the template compiler or what I did before was not supposed to work.
Funny thing is that these "errors" appear only sometimes. I can't seem to understand when they appear and when they don't.
The text was updated successfully, but these errors were encountered: