Add failing test around moduleName mutation #14196
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[ci skip]
Normalize attribute assertions and behavior across htmlbars and glimmer Remove _defaultTagName which was for support of legacy each AST plugin which was removed Some general cleanup (cherry picked from commit e447dd8)
(cherry picked from commit fcd906e)
(cherry picked from commit 5327eb0)
(cherry picked from commit e176cff)
Glimmer requires that a number of factories and injections added by `ApplicationInstance#setupRegistry` are present, but the testing harness only has access to calling `Ember.Application.buildRegistry` which leaves the registry in a partially populated state. This exposes `Ember.ApplicationInstance.setupRegistry` so that `ember-test-helpers` can ensure that its registry is properly populated. (cherry picked from commit ffc29cd)
In emberjs#13829, we introduced the ability to pass a custom key function for the cache. However, we are incorrectly passing `(key, value)` to `set` when the function expects `(obj, value)`. This is causing problems for caches that uses a custom cache keying function (such as Glimmer's compilation cache), because the get side would handle the keying correctly, but the set side would compute the key based on the already-computed key (i.e. `this.key(this.key(obj))`). In Glimmer's compilation cache, this is causing the cache to always miss because the `set` side would cache everything under the `"undefined"` key. (cherry picked from commit 80f0462)
The deprecation before: ``` `Ember.Binding` is deprecated. Consider using an `alias` computed property instead. ``` The deprecation after: ``` The `twoWayTest` property of `<(subclass of Ember.Component):ember483>` is an `Ember.Binding` connected to `direction`, but `Ember.Binding` is deprecated. Consider using an `alias` computed property instead. ``` (cherry picked from commit 8601462)
(cherry picked from commit f015b6e)
(cherry picked from commit 86e1606)
Addresses emberjs#13494 (cherry picked from commit 4a81b5f)
(cherry picked from commit 0a85708)
[ci skip]
(cherry picked from commit a931cb0)
(cherry picked from commit 98269f8)
(cherry picked from commit 47c6a6f)
This fixes a failing test introduced in the previous test where Engines rendered even though shouldRender was set to false. (cherry picked from commit d2d6e27)
(cherry picked from commit ddd7f58)
Currently, `fillIn` sets the new value to all matches, though the events are only fired for the first one, making Ember only aware of that one. This can cause a problem while trying to _debug_ a test. Fixes emberjs#14018 (cherry picked from commit 7e9176d)
(cherry picked from commit 61354f0)
From [HTML5, A vocabulary and associated APIs for HTML and XHTML](https://www.w3.org/TR/html5/infrastructure.html#boolean-attributes): A number of attributes are boolean attributes. The presence of a boolean attribute on an element represents the true value, and the absence of the attribute represents the false value. **Note:** The values "true" and "false" are not allowed on boolean attributes. To represent a false value, the attribute has to be omitted altogether. Fixes emberjs#14024 (cherry picked from commit 5d2175a)
As suggested in emberjs#14022, `setEach` is a more ergonomic, not more efficient method. Fixes emberjs#14022 (cherry picked from commit 8514612)
The second parameter is an object of the query parameters currently set on the URL, not an integer. (cherry picked from commit 26c8ec6)
Before:
```
Route.reopen({
replaceWith: function () {
for (var _len3 = arguments.length, args = Array(_len3), _key3 = 0; _key3 < _len3; _key3++) {
args[_key3] = arguments[_key3];
}
return this._super.apply(this, prefixRouteNameArg.call.apply(prefixRouteNameArg, [this].concat(args)));
},
transitionTo: function () {
for (var _len4 = arguments.length, args = Array(_len4), _key4 = 0; _key4 < _len4; _key4++) {
args[_key4] = arguments[_key4];
}
return this._super.apply(this, prefixRouteNameArg.call.apply(prefixRouteNameArg, [this].concat(args)));
},
...
});
```
After:
```
Route.reopen({
replaceWith: function () {
for (var _len3 = arguments.length, args = Array(_len3), _key3 = 0; _key3 < _len3; _key3++) {
args[_key3] = arguments[_key3];
}
return this._super.apply(this, prefixRouteNameArg(this,args));
},
transitionTo: function () {
for (var _len4 = arguments.length, args = Array(_len4), _key4 = 0; _key4 < _len4; _key4++) {
args[_key4] = arguments[_key4];
}
return this._super.apply(this, prefixRouteNameArg(this,args));
},
...
});
```
Which is more readable and avoid a needless allocation from the `concat`
(and presumably `.call.apply` is rather inefficient).
As a bonus, it also removes the splatting in `prefixRouteNameArg`.
(cherry picked from commit 40c37b6)
(cherry picked from commit e0680ca)
`mut` documentation contains code that does not work. It also references a blog post with outdated information. This PR fixes the code with the right behaviour and remove the link to avoid confussion. Fixes emberjs#14057 (cherry picked from commit 436c330)
[ci skip]
[ci skip]
(cherry picked from commit 0f25156)
When the toplevelView (aka `ownerView`) is being destroyed (during test
teardown generally) an error was ocurring under certain "special"
circumstances. Specifically, if removal of a sibling node triggered a
revalidation of one still attached.
Consider the following example:
```hbs
{{#x-select as |select|}}
{{#select.option value="1"}}{/select.option}}
{{#select.option value="2"}}{/select.option}}
{{/x-select}}
```
The components in question are using the registration pattern so that
the children notify/register with the parent upon `didInsertElement`
(and `willDestroyElement`).
When a new option is added or removed from the parent, it updates its
`value` property which is bound to each options `selected`
attribute (to toggle the selection state in the DOM).
The specific mechanism that causes the DOM to get updated when the
various computed properties change is that a Stream object calls
`ownerNode.emberView.scheduleRevalidate`. This tells the rest of the
system to start walking the tree to flush out any updates to the DOM.
When a view is being removed, we set the `emberView`
property of its `renderNode` to `null` before traversing the subtree of
children. This means, that as the children are removed (and therefore
cause the `value` of the parent to change) the `selected` attribute
binding attempts to call `ownerNode.emberView.scheduleRevalidation()`.
Unfortunately, when we are actually tearing down the
`ownerNode.emberView` itself that invocation results in an error (
cannot call `scheduleRevalidation` on `null`).
---
This change adds a simple guard to avoid calling `scheduleRevalidation`
when the `ownerNode.emberView` is being torn down.
(cherry picked from commit 2fa4015)
This PR makes explicit that render only accepts a live template as its `into` option. Fix emberjs#14046 (cherry picked from commit 2236950)
This is necessary for lazily-loaded routes where the handler is not available synchronously. This includes events like loading and queryParamsDidChange which trigger synchronously after starting a transition, in those cases we should by-pass handlers that are not yet loaded. (cherry picked from commit ce72c7e)
…nents. (cherry picked from commit 64a6853)
Provides some temporary relief for emberjs#14114. (cherry picked from commit 0c2a3f0)
During each `Ember.Route`'s `willDestroy` we trigger a `run.once(this.router, '_setOutlets');`. This is so that the routes views are destroyed properly (by removing them from the outlet state). During `Ember.Router`'s `willDestroy` we clear `this._toplevelView` (along with a bunch of other cleanup). These two things combined can mean that `this._toplevelView` is `null` when `_setOutlets` is called again (during the `Router`'s destruction). In that scenario we are actually creating another `this._toplevelView` and setting up another rendered root (since one doesn't exist). When this happens the newly created root is never cleaned up, since the Router's `willDestroy` has already ran and can no longer clean up after itself. (cherry picked from commit 890fb04)
…ces. An engine instance needs to share the following with its parent: * `service:-glimmer-environment` (only for glimmer) * `renderer:-dom` or `renderer:-inert`, depending on whether the environment is interactive (cherry picked from commit 79026a7)
Ensure that engines are destroyed _before_ top-level views so that top-level views are not re-instantiated during engine teardown. (cherry picked from commit 447df33)
Prior to this `_environment` could not be relied upon in routes or views (which is used when `shouldRender: false` is used with `visit`). The refactor uses the environment to determine what injections to setup in the engine-instance, and has the `application-instance` defer to the `engine-instance` to avoid duplication. (cherry picked from commit 3de2360)
Prior to this change we were defaulting to `Ember.Component` which does not receive injections (which meant it was missing the `renderer`). This worked "ok" in HTMLBars since we created a default global renderer to handle components/views without a container/owner. This change ensures that in the template only component case we fall back to using the default component looked up from the owner. This ensures that injections are present. (cherry picked from commit 831c8cb)
(cherry picked from commit d78c0dd)
[ci skip]
`document.contains` does not exist in Internet Explorer (tested in IE9, IE10, and IE11), but `document.body.contains` does. Since `document.body.contains` works just fine for our purposes (we don't need to check if `document.head` contains the element), we can just use that.
[BUGFIX beta] Fix cleanup
[ci skip]
|
☔ The latest upstream changes (presumably a40e8de) made this pull request unmergeable. Please resolve the merge conflicts. |
|
I believe this has been addressed, closing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Failing test that addresses #14192
Beta version of #14194
cc @krisselden @rwjblue @trentmwillis