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

adjust assertion to allow for either undefined or null values #16485

Closed
wants to merge 51 commits into from

Conversation

Dhaulagiri
Copy link
Contributor

Based on conversation with @rwjblue in #16148 he suggested adjusting this assertion to allow value to be null. I confirmed locally that this fixes the instances of #16259 we are seeing which we are unable to correct with the upstream Ember Data fix just yet.

If this looks good I can adjust the commit message as needed, but ideally this would be a [BUGFIX Release].

rwjblue and others added 30 commits February 14, 2018 09:45
Previously, any errors thrown during `didInsertElement` would leave the
running environment in an invalid state (`env.inTransaction` would be
`true` but `this._transaction` would have been nullified).

This commit ensures that we _always_ reset `inTransaction` if
`Environment.prototype.commit` is called. Thus avoiding an error RE:
"calling commit on null"...

(cherry picked from commit ab4a2e3)
`Ember.trySet` is designed to be an error-tolerant form of `Ember.set`
(per its public API documentation). However, in some circumstances an
error is still thrown when setting on a destroyed object.

Prior to this change, the following would properly prevent an error:

```js
let obj = { some: { deep : 'path', isDestroyed: true }};

Ember.trySet(obj, 'some.deep', 'stuff');
```

But the following would throw an error incorrectly:

```js
let obj = { some: 'path', isDestroyed: true };

Ember.trySet(obj, 'some', 'stuff');
```

This fixes the latter case...

(cherry picked from commit 1f4a3bc)
Prior to this change, if `Ember.get` (or `this.get`) is used to access a
property that is an Ember.Object with an `unknownProperty` that always
returns a non-`undefined` value, the internals of `Ember.get` would
check if that property is a "descriptor" and (due to the
`unknownProperty` never returning `undefined`) the dev time assertion
for accessing a proxy's content without using `Ember.get` would be
triggered.

(cherry picked from commit 4e950a9)
The current sanitizer regexp works well when there is a single invalid
character, but does not handle the case where multiple invalid chars are
present (e.g. with a branch name of
`__DESCRIPTOR__-something__something/special`).

(cherry picked from commit 2e99d1f)
Fixes an issue with static numbers > 4000 being used in a template.

(cherry picked from commit 5bb4b66)
Main changes:

* ES6ify a few classes.
* Fix issue when calling `.replaceWith` when the current transition is also
  a `.replaceWith` (previously this would surprisingly result in a
  `.transitionTo` _not_ a replace).

[Full
changelog](tildeio/router.js@v2.0.0-beta.1...v2.0.0-beta.2)

(cherry picked from commit f470727)
The "symbols" we generate and use internally in Ember are not really
`Symbol`s (yet) so they do not get automatically "whitelisted" by the
pre-existing `typeof property === 'symbol'` check.

This change ensures that any of our "internal symbols" are allowed to be
looked for without `.get`.

(cherry picked from commit ea2590b)
Before this PR, having a component with an `{{input}}` with bound type
would make that input only to work the first time it was rendered. This
was because the first param would be shifter from params, and that would
modify the params of the rest of the instances.

Fixes emberjs#16256

(cherry picked from commit 51f09b5)
This reverts commit ecd93f9.

(cherry picked from commit e1671a0)
…nts"

This reverts commit b861ccc.

(cherry picked from commit ad5cdf6)
In older Ember versions the default components were generally labelled
as `Ember.Component` or `Ember.TextField` via the `NAME_KEY` property on
the class itself. During the glimmer-vm upgrade which landed during the
3.1 canary cycle the custom `NAME_KEY` was lost for `Ember.TextField`.

This commit removes needless usage of `NAME_KEY` (not needed since our
normal `Ember.Object` instance's `.toString` methods check for the
factories own `toString` properly), and updates the actual paths to
match the new JS modules API locations (thus reducing more "globals"
shenanigans).

(cherry picked from commit 3462932)
Kenneth Larsen and others added 18 commits March 12, 2018 12:06
In general,Ember's testing system leverages `ajaxSend` and
`ajaxComplete` events to know when `jQuery.ajax` requests are pending
(and it should therefore "pause settledness" of the testing helpers like
`click` / `andThen` / etc).

In the Ember 3.1 cycle Ember changed from using `jQuery.on` to
`document.addEventListener` to subscribe to these events. Unfortunately,
`jQuery.ajax` **does not** trigger listeners added in this way (we would
have had to do `document.onajaxSend = function() { /* stuff here */ }`).
The result of this change is that "normal" acceptance testing helpers
will _no longer_ be able to detect and wait for pending `jQuery.ajax`.

So, for example, you might expect that given the following

```js
await click('#some-selector');

assert.equal(
   $('#something-that-should-exist-after-an-ajax-call').text(),
   'hi'
);
```

the click helper would wait until `jQuery.ajax` calls were settled
before the assertion, but prior to the changes in this commit
the promise returned by `click` would **not** wait until the
`jQuery.ajax` request was completed and therefore the assertion would
fail.

---

This commit reverts the changes to use `document.addEventListener` back
to using `jQuery.on` directly when `jQuery` is available.

Many bothans died to bring us this information. 😭 😢 :crying_cat:

Paired with @rwjblue to resolve this issue.

(cherry picked from commit 8609228)
Resolve emberjs#14741.

(cherry picked from commit dfaa867)
* helper-addon
* initializer-addon
* instance-initializer-addon

(cherry picked from commit 5c6eef3)
* helper
* initializer
* instance-initializer

(cherry picked from commit 14d5bb0)
Also fix missing apostrophe in router examples.
Fixes emberjs#16394

(cherry picked from commit 428246f)
The fact that this private property was named router was preventing user
from injecting the router service into routes like this:

```js
export default Route.extend({
  router: service()
});
```

I consider that the above code is pretty reasonable, and it should be
the private property the one that should be renamed to a clearer `_router`
name that is less likely to collide with user-defined properties or methods.

If you deem this an _intimate_ API we could consider adding a deprecated
alias, but I don't have evidence to consider it such, as `router` is too
common of a term to search for in emberobserver.

(cherry picked from commit 4690f63)
@rwjblue
Copy link
Member

rwjblue commented Apr 11, 2018

Seems good to me, [BUGFIX release] seems like a good target...

@Dhaulagiri
Copy link
Contributor Author

@rwjblue i also targeted the release branch because master wouldn't let my app boot for reasons I didn't look into, is that a valid target as well?

@rwjblue
Copy link
Member

rwjblue commented Apr 11, 2018

Sadly due to somewhat arcane release processes (that are completely my fault) we need to target this PR at master, then cherry-pick (or land separate PRs) into beta and release.

@Dhaulagiri Dhaulagiri changed the base branch from release to master April 11, 2018 19:19
@Dhaulagiri
Copy link
Contributor Author

I'm going to re-open this since I kinda screwed up my branches over here

@Dhaulagiri
Copy link
Contributor Author

Moved to #16494

@Dhaulagiri Dhaulagiri deleted the release branch April 11, 2018 19:23
@rwjblue
Copy link
Member

rwjblue commented Apr 11, 2018

Sorry for the run around @Dhaulagiri, thank you!

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.