Ember 4.0 compatibility - drops support for Ember < 3.28 #717
Conversation
fsmanuel
force-pushed
the
ember-4-compatibility
branch
7 times, most recently
from
926bd77 to
986eb89
Compare
fsmanuel
force-pushed
the
ember-4-compatibility
branch
from
067cba5 to
9ab10fd
Compare
fsmanuel
force-pushed
the
ember-4-compatibility
branch
from
9ab10fd to
bcf499c
Compare
|
@tehhowch wanna do another review? |
fsmanuel
changed the title
| this._super(...arguments); | ||
| get(this, 'validations').destroy(); | ||
| this.validations.destroy(); | ||
| }, |
There was a problem hiding this comment.
I always feel the call to super in "unwinding" contexts should be last, not first.
| let whiteList = makeArray(options.on); | ||
| let blackList = makeArray(options.excludes); | ||
|
|
||
| let validationResults = get(this, 'validatableAttributes').reduce( | ||
| (v, name) => { | ||
| if (!isEmpty(blackList) && blackList.indexOf(name) !== -1) { | ||
| return v; | ||
| } | ||
|
|
||
| if (isEmpty(whiteList) || whiteList.indexOf(name) !== -1) { | ||
| let validationResult = get(this, `attrs.${name}`); | ||
| let validationResults = this.validatableAttributes.reduce((v, name) => { | ||
| if (!isEmpty(blackList) && blackList.indexOf(name) !== -1) { |
There was a problem hiding this comment.
We can rename these to e.g. allowed and forbidden.
The indexOf checks should be replaced by includes, too
|
@fsmanuel i was able to pull this down and run unit & integration tests for some just-migrated-to-3.28 apps, without issues. We have more extensive test suites but they are more costly to run, so they'll need to wait until this PR is released in 5.0 I'm curious: does this PR drop support for Ember 3.24 just because, or are there things only in 3.28 or later that are necessary for the current implementation? |
|
As another data point (as someone running a large ember app in production on the qonto@3767e41 fork). |
|
@MelSumner thanks for approving! @tehhowch I think I needed to drop support for 3.24 but I'm not 100% sure. The general problem with old versions is that the maintaining efforts for the test coverage increases a lot. For example ember bootstrap was used in this addon for the docs and then you need to figure out which ember, addon and ember bootstrap versions work well together. |
|
Released as |
Breaking changes:
First attempt to make it ember 4.0 compatible.
volatile()from the code baseThanks to @zeppelin and @qonto I could cherry pick a lot from the fork.
ref #716
closes #701