Skip to content
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

Open
boris-petrov opened this issue Feb 10, 2021 · 36 comments
Open

Comments

@boris-petrov
Copy link
Contributor

boris-petrov commented Feb 10, 2021

Running ember serve with ember-source 3.25 leads to:

unexpectedly found "!~" when slicing source, but expected ""
unexpectedly found " comment ~" when slicing source, but expected " comment "

These correspond to these two lines of HBS (they are in different files, I've just put them here for conciseness):

{{~!~}}
{{! comment ~}}

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.

@urbany
Copy link

urbany commented Feb 10, 2021

I was also seeing these errors after upgrading to 3.25.1 but it stoppped after i deleted node_modules and installed it again

@rwjblue
Copy link
Member

rwjblue commented Feb 11, 2021

Hmm, this is odd indeed. It definitely sounds like a bug, but I'm not sure exactly what is going on.

@rwjblue
Copy link
Member

rwjblue commented Feb 11, 2021

I could definitely use a reliable reproduction if someone has the time to try and isolate one...

@boris-petrov
Copy link
Contributor Author

boris-petrov commented Feb 11, 2021

@rwjblue - here. Clone, npm install, ember serve. You'll see the warnings in the console. I'm running Node v15.7.0.

P.S. There are obviously some caches at play because the second time I run ember serve the errors are gone. Even if I remove the dist directory. Perhaps something is left in /tmp, not sure. You can probably tell.

This is the HBS that used to cause the warnings:

{{! comment ~}}
<div></div>

<div>
  {{~!~}}
  <span></span>
</div>

@rwjblue
Copy link
Member

rwjblue commented Feb 11, 2021

Ya, we cache the compilation result into system temp. You can disable that behavior (likely causing an error on every rebuild) with CI=true environment variable.

@boris-petrov
Copy link
Contributor Author

Well, in any case, that's concerning you mostly because you're fixing it. 😃 You managed to reproduce it, right?

@rwjblue rwjblue added this to the 3.25 milestone Feb 16, 2021
@Turbo87
Copy link
Member

Turbo87 commented Feb 22, 2021

FWIW we're seeing similar issues:

unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<form" when slicing source, but expected "class"
unexpectedly found "<form" when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<asid" when slicing source, but expected "class"
unexpectedly found "<asid" when slicing source, but expected "class"
unexpectedly found "<asid" when slicing source, but expected "class"
unexpectedly found "<asid" when slicing source, but expected "class"
unexpectedly found "<asid" when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"
unexpectedly found "<div " when slicing source, but expected "class"

interestingly we are only seeing it with class and I'm wondering if it is related to us using ember-css-modules.

@rwjblue
Copy link
Member

rwjblue commented Feb 22, 2021

@Turbo87 - What is the template snippet that causes that output?

@Turbo87
Copy link
Member

Turbo87 commented Feb 22, 2021

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.

@rwjblue
Copy link
Member

rwjblue commented Feb 22, 2021

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)?

@Turbo87
Copy link
Member

Turbo87 commented Feb 22, 2021

Are these actual errors or is it console output during template compilation?

just console output during the build steps.

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)?

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.

@Turbo87
Copy link
Member

Turbo87 commented Feb 25, 2021

@rwjblue if you need a repro, the crates.io codebase is currently showing these warnings too :)

@rwjblue
Copy link
Member

rwjblue commented Feb 25, 2021

OK, awesome. That is helpful, thank you!

@boris-petrov
Copy link
Contributor Author

I believe this issue has been resolved in the past weeks/months - I don't seem to see the warnings any more. @Turbo87 - am I right? @rwjblue - any ideas when this was fixed? :)

@nelstrom
Copy link

@boris-petrov which version of Ember are you using? We are on ember-source version 3.26.1 and we still get these messages. I'm curious whether upgrading to 3.27.x might silence these messages?

@boris-petrov
Copy link
Contributor Author

@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.

@nelstrom
Copy link

@boris-petrov ok, that gives me some motivation to upgrade.

@boris-petrov
Copy link
Contributor Author

I started getting these errors again after updating ember-cli-htmlbars from 6.0.0 to 6.0.1 (as well as a bunch of other ones). So I guess the issue hasn't been resolved after all. Any ideas?

@bertdeblock
Copy link
Member

I'm seeing similar logs when updating ember-cli-htmlbars to 6.0.1:

unexpectedly found "<@Subse" when slicing source, but expected "@source"

The @source argument comes from an AST transform in ember-freestyle:
https://github.com/chrislopresto/ember-freestyle/blob/master/lib/ast-transform.js#L91

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!

@boris-petrov boris-petrov reopened this Dec 8, 2021
@basz
Copy link

basz commented Dec 16, 2021

I'm noticing a few [Babel: @xxxx/xxxx > applyPatches]unexpectedly found "<div " when slicing source, but expected "class" instances.

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

@dfreeman
Copy link
Contributor

I hit this in a repo that uses ember-css-modules the same way @Turbo87 described. It definitely seems plausible it's related to the template transform happening in that package, or from the other comments here from template transforms in general

@nruth
Copy link
Contributor

nruth commented Mar 8, 2022

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?):

node_modules/ember-source/dist/ember-template-compiler.js

Screenshot 2022-03-08 at 02 58 57

This looks different to the following similar code, which I guess is to do with different javascript ecosystems/modules/packagers.

image

image

@bertdeblock
Copy link
Member

@nruth I think that's just generated code in development, those DEBUG blocks are normally stripped away in production builds if I'm not mistaken.

@dfreeman
Copy link
Contributor

dfreeman commented Mar 9, 2022

I poked at this a bit today, and it looks like adding any synthetic AttrNode in a template transform will trigger this.

I don't have great in-depth knowledge of the AST normalization process, but these lines seem like they're treating the loc of the synthetic node as a real location that they can read from in source, which is triggering the warning here.

@rwjblue
Copy link
Member

rwjblue commented Mar 10, 2022

Makes sense, thanks for digging into that @dfreeman. I think the next thing to do is to avoid that warning code path when working on synthetic AST nodes...

@runspired
Copy link
Contributor

Also hit this when working to modernize flexi (which produces synthetic Attr nodes). I went down the rabbithole of adjusting loc because it otherwise spits out thousands of these.

bors added a commit to rust-lang/crates.io that referenced this issue Mar 23, 2022
…bo87

Update dependency ember-css-modules to v2.0.1

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](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) | [![age](https://badges.renovateapi.com/packages/npm/ember-css-modules/2.0.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/ember-css-modules/2.0.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/ember-css-modules/2.0.1/compatibility-slim/2.0.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/ember-css-modules/2.0.1/confidence-slim/2.0.0)](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#&#8203;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#&#8203;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).
@colenso
Copy link

colenso commented Nov 29, 2022

Is this an issue with glimmer.js? If yes, will it be fixed in 2.0?

@runspired
Copy link
Contributor

its a bug in the sense that it outputs a meaningless warning, its not actually an issue beyond that

@navels
Copy link

navels commented May 24, 2023

This is also impacting consumers of the ember-basic-dropdown addon (which uses ember-maybe-in-element which creates synthetic attributes). I don't really see how to work around this in that addon as ember-css-modules was able to.

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 loc.source:

newAttr.loc = {
  source: '(synthetic)',
  start: { line: 1, column: 0 },
  end: { line: 1, column: 0 }
};

and key off that . . . ?

@navels
Copy link

navels commented May 26, 2023

PR for fixing the {{~!}} case: glimmerjs/glimmer-vm#1430

PR for fixing the remaining cases: https://github.com/glimmerjs/glimmer-vm/pull/1431/files

@yuriyaran
Copy link

PR for fixing the {{~!}} case: glimmerjs/glimmer-vm#1430

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?
I'd be happy to have this merged - since console warnings are indeed distracting

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented Jun 18, 2023

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!

@navels
Copy link

navels commented Jun 18, 2023

Thanks, I will stay tuned. Happy to retest and update the PRs if they are still warranted.

@KoKuToru
Copy link

Any updates on this?
I am adding synthetic AttrNode to every ElementNode and get hundreds of messages in the console,
pretty anoying.

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 loc that would hide the messages without referencing a existing slice of the original source.
Changing the source or overwriting the source is not possible..

@navels
Copy link

navels commented Jan 8, 2024

Any updates on the refactoring that was blocking this fix?

@NullVoxPopuli
Copy link
Sponsor Contributor

Someone probably needs to do the work in @glimmer/syntax

CvX added a commit to discourse/discourse that referenced this issue May 22, 2024
e.g. `unexpectedly found "! no whitespace ~" when slicing source, but expected " no whitespace "`

See: emberjs/ember.js#19392
CvX added a commit to discourse/discourse that referenced this issue May 22, 2024
e.g. `unexpectedly found "! no whitespace ~" when slicing source, but expected " no whitespace "`
See: emberjs/ember.js#19392

Co-authored-by: David Taylor <[email protected]>
backspace added a commit to cardstack/boxel that referenced this issue May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests