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

Bump all glimmer-vm dependencies to 0.87.1 #20609

Merged
merged 6 commits into from
Jan 2, 2024

Conversation

NullVoxPopuli
Copy link
Contributor

Contains some perf improvements as well as a memory leak fix for a feature we introduced in 3.25, described here: https://guides.emberjs.com/release/in-depth-topics/rendering-values/

Here is the changelog: https://github.com/glimmerjs/glimmer-vm/releases/tag/v0.86.0

@NullVoxPopuli
Copy link
Contributor Author

types are failing because types are failing on main

@@ -32,18 +31,6 @@ moduleFor(
);
}

'@test SUPPORTS_EVENT_OPTIONS is correct (private API usage)'(assert) {
Copy link
Contributor Author

@NullVoxPopuli NullVoxPopuli Dec 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this test anymore.
glimmerjs/glimmer-vm#1535

From the implementation notes:

/*
  Internet Explorer 11 does not support `once` and also does not support
  passing `eventOptions`. In some situations it then throws a weird script
  error, like:
  ```
  Could not complete the operation due to error 80020101
  ```
  This flag determines, whether `{ once: true }` and thus also event options in
  general are supported.
*/

https://github.com/glimmerjs/glimmer-vm/pull/1535/files#diff-301b4fe274bd34f5552c35fc1f58078575896ba487df721bed939eb1d0fe9891L19-L30

@NullVoxPopuli NullVoxPopuli changed the title Bump all glimmer-vm dependencies to 0.86 Bump all glimmer-vm dependencies to 0.87.1 Dec 31, 2023
@ef4
Copy link
Contributor

ef4 commented Jan 2, 2024

This is now passing after I removed ts@next. The PR will still show that as an expected check and I don't think you can fix that without reopening the PR, so I'm just going to ignore that check.

@ef4 ef4 merged commit 5c19329 into emberjs:main Jan 2, 2024
20 checks passed
@NullVoxPopuli NullVoxPopuli deleted the upgrade-to-glimmer-vm-0-86 branch January 2, 2024 20:07
Comment on lines +61 to +63
// SAFETY: `get` could not infer the type of `prop` and just gave us `unknown`.
// we may want to throw an error in the future if the value isn't string or null/undefined.
let elementId = get(component, prop) as string | null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N.b. the right way to do this is to use a dev-time assert() that is stripped in prod builds instead!

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

Successfully merging this pull request may close these issues.

3 participants