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

2.2.2 breaking change: globals are now 'not in scope' for templates? #38

Open
davidtaylorhq opened this issue Apr 18, 2024 · 13 comments
Open

Comments

@davidtaylorhq
Copy link

There appears to be a breaking change between 2.2.1 and 2.2.2. Previously, in gjs files, we could reference JS globals directly as helpers. For example, we were using @something=(Boolean this.blah) to cast a value to a boolean.

Now, we get the error:

Error: Attempted to resolve a helper in a strict mode template, but that value was not in scope: Boolean

Adding const Boolean = window.Boolean to the file works around the problem.

This problem is not limited to helpers. Something like {{document.title}} previously worked, but now throws a 'not in scope' error.

@ef4
Copy link
Contributor

ef4 commented Apr 18, 2024

LIke #38, this was intentional because in general there's no way to statically disambiguate between javascript globals and future ember keywords. That is the tradeoff of not making people import keywords.

We do support globalThis, so I would expect @something=(globalThis.Boolean this.blah) to work.

We can decide to support a standard list of platform-provided globals, including Boolean and document. Perhaps we should do that right away to mitigate the impact of this change.

The thing we don't want to do is support arbitrary globals. Like, if somebody decides they don't like importing their component all the time and tries window.CustomButton = CustomButton, that doesn't make <CustomButton /> work in all components.

@chancancode
Copy link
Member

Another (IMO more important) consideration is html tags, or rather, for the scoping rule to work consistently across all positions (<___> vs {{___}}); say we support window and document, if HTML later introduces <window> or <document> elements, there will be no way to invoke them (without making convoluted excepts to the scoping rules)

@chancancode
Copy link
Member

Not to say that I am against supporting window or document for that reason, but we should keep that in mind every time we consider adding to that list

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Apr 19, 2024

I would like common casts added to the list: Boolean, String (helpful for glint)

And utility namespaces: Math, JSON, Date, URL

And Common globals: window, document, navigator, localStorage

Tho, given the congern ef4 raises, i can deal with destructuring globalThis, as awkward as it is. Tho, elements must all be lowercase, so the namespaces and casts seem safe

@Techn1x
Copy link

Techn1x commented Oct 1, 2024

this was intentional

breaking but marked as a patch?

Was wanting to consume this 2.2.2 patch to fix ember-cli/ember-template-imports#229 but as Simon mentions it's got other regressions like this.


Likely related, I'm also having issues with enums that _are_ named and imported (click to expand)
// app/components/alert.gts
export enum AlertType {
  Information = 'information',
  Warning = 'warning',
  Critical = 'critical',
  Success = 'success',
  NewFeature = 'new-feature',
}

export component Alert: TOC = <template>
  {{#if (eq @type AlertType.Warning)}}
    ...
  {{else if ...}}
    ...
  {{/if}}
</template>

results in error at runtime

Uncaught (in promise) Error: Attempted to resolve a value in a strict mode template, but that value was not in scope: AlertType

EDIT: opened issue for enums #65

@ef4
Copy link
Contributor

ef4 commented Oct 1, 2024

Globals were not supposed to work, it was a bug that they worked, the bug is dangerous because if people come to rely on it, Ember is prevented from ever adding a new keyword again, because the template compiler has no way of distinguishing it from an in-scope global.

Enums may be a different bug, please file a separate issue for that one.

@Techn1x
Copy link

Techn1x commented Oct 1, 2024

Enums may be a different bug, please file a separate issue for that one.

Thanks, I've put the bug example behind an expand block for the time being (and simplified the example as well)

@Techn1x
Copy link

Techn1x commented Oct 1, 2024

Globals were not supposed to work

That's news to me, and IMO likely not known by anyone outside of the core devs 😅

I think anyone using any number of GTS files are likely to be using globals all over the place. Especially so given typescript is totally happy with it.

the bug is dangerous because if people come to rely on it, Ember is prevented from ever adding a new keyword again, because the template compiler has no way of distinguishing it from an in-scope global.

I agree, though it being a patch (and without mentioning it in the changelog at all......) might have been misguided. But having now looked over other GH issues on the repo regarding 2.2.2, I suspect that has already been made clear by the number of issues reported, and I shouldn't have brought it up.

@Techn1x
Copy link

Techn1x commented Oct 4, 2024

I would like common casts added to the list: Boolean, String (helpful for glint)

Array would be a good one for that list

I just saw a colleague use @chartMargin={{Array 10 20 10 30}} and it blew my mind. And here I have been importing & using array ember helper like a chump, when the native constructor is clearly better 😄

@NullVoxPopuli
Copy link
Contributor

Woah, yes

@NullVoxPopuli
Copy link
Contributor

turns out our work around doesn't work for Infinity, as it cannot be redefined.

so I have to do const BIGGEST_VALUE = Infinity instead and use BIGGEST_VALUE in my <template> part

@ef4
Copy link
Contributor

ef4 commented Nov 26, 2024

Wouldn't {{globalThis.Infinity}} be a workaround?

@NullVoxPopuli
Copy link
Contributor

it would, yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants