breaking: upgrades to Ember 5.3 / drops Ember 4.4

[Pixelik]

Resolves #733

Chore

• runs ember-cli-update --to 5.3
• updates to ember-data 5.3
• runs yarn & updates yarn.lock
• installs babel-plugin-dynamic-import-node
  • otherwise the ember 4.x test scenarios throw Error
• sets stylelint to target .scss files
• runs yarn lint:fix
• fixes stylelint errors
• installs @babel/plugin-proposal-private-property-in-object
  • otherwise ember s throws Error
• installs @ember/string

Refactor

• replaces PromiseManyArray.filter with PromiseManyArray.content.filter
  • because PromiseManyArray.filter no longer exists in Ember 5
• stops using A() with PromiseManyArray
  • as it is no longer allowed in Ember 5
• increases later() timeout for some tests
  • otherwise the following tests sometimes fail:
    • Integration | Validations | Factory - General: debounced validations
    • Integration | Validations | Factory - General: debounced validator should only be called once
• refactors isDescriptor() logic & syntax
  • otherwise the embroider test scenarios are failing

Breaking

• drops support for Ember 4.4
  • because we only support the last 2 LTS version anyway & because the relevant Ember 4.4 test scenario throws Error: required module @ember/renderer is missing
• drops support for Node v14

Tasks:

• Added test case(s)
• Updated documentation

[Pixelik]

PromiseManyArray.filter no longer exists in Ember 5 but I'm not sure if this is an acceptable solution.

[fsmanuel]

@runspired can you help?

[fsmanuel]

I found

emberArray(value.toArray().filter((v) => isValidatable(v)));

on the qonto fork qonto@82a5816

[runspired]

folks need to resolve the promise array upfront, either by awaiting or by using record.hasMany('foo').value() to get it synchronously

[fsmanuel]

@Pixelik I'll do a review in the next days!

[Pixelik]

Thank you @fsmanuel 🙏

[Pixelik]

did you get a chance to have a look @fsmanuel ? 🙂 it would be nice to make a v6 release asap 🙏

[Pixelik]

@MelSumner do you think we could merge this and make a release? 🙏

[fsmanuel]

@Pixelik sorry to keep you waiting so long. I finally found the time to do a review. Thanks for all the work you put into this!

I looked into the decorator/descriptor topic:

• Support ember 3.10, drops support for decorators #641 added the code to make it work for ember >= 3.10
• Support Ember 3.13 (redux) #668 made it work for ember >= 3.13
• 89d8307 I changed it to an import

So it seems this internal import is stable since ember 3.13 and we no longer need the fallbacks. More in the comments.

I'm a little hesitant to have a breaking release without a proper version that support ember until 4.12. So my suggestion would be we cherry pick the decorator changes and I'll do the ember-cli update to 4.12 tomorrow. After that is released you can rebase and we release a new major version. What do you think?

[Pixelik]

Hello @fsmanuel, thanks for the review 🙏

Could you please add commit/code suggestions instead ? I'm not sure I understand what changes you'd like me to make.

Please note that some of my refactoring is needed for the embroider scenarios to pass.
You will probably face the same issues if you try to run those embroider tests on that other PR you created. Up until now you've had those embroider tests deactivated, not sure why, but my PR reactivates them because they are necessary moving to Ember 5.

Also, this PR runs tests on Ember 4.12, so I'm not sure why we need a separate release that has ember-cp-validations on 4.12 since this one has it on 5.3 but tests against 4.12 are passing just fine, but I'm fine with it either way 🤷‍♂️

[fsmanuel]

Could you please add commit/code suggestions instead ? I'm not sure I understand what changes you'd like me to make.

Sure thing! Let's take the current isDescriptor function:

// >> This internal import is stable since 3.13
import __EMBER_METAL__ from '@ember/-internals/metal/index';

export function isDescriptor(o) {
  // >> This is always true since 3.13
  if (__EMBER_METAL__ && __EMBER_METAL__.isClassicDecorator) {
    return __EMBER_METAL__.isClassicDecorator(o);
  // >> This will never be reached
  } else {
    return (
      o && (typeof o === 'object' || typeof o === 'function') && o.isDescriptor
    );
  }
}

so we can refactore it to:

import { isClassicDecorator } from '@ember/-internals/metal/index';

export function isDescriptor(o) {
  return isClassicDecorator(o);
}

The same goes for getDependentKeys. With your new features -private/ember-internals.js could be rewritten to something like that:

import {
  descriptorForDecorator,
  isClassicDecorator,
} from '@ember/-internals/metal/index';

export function getDependentKeys(descriptorOrDecorator) {
  const descriptor = descriptorForDecorator(descriptorOrDecorator);
  // TODO: Figure out why the fallback `|| [descriptor.altKey]` is needed
  return descriptor._dependentKeys || [descriptor.altKey];
}

// New decorator implementations that are not detected by isClassicDecorator()
// TODO: Figure out if there is an alternative to isClassicDecorator + this exceptions
const DECORATOR_IMPLEMENTATIONS = [
  'AliasDecoratorImpl',
  'ComputedDecoratorImpl',
];

export function isDescriptor(o) {
  return (
    isClassicDecorator(o) ||
    DECORATOR_IMPLEMENTATIONS.includes(o.constructor?.name)
  );
}

Now I'm currious how the use of AliasDecoratorImpl and ComputedDecoratorImpl looks like in code. Can you add an example or link to your addon where I can see it?

I would also appreciate if we could address this change in a separate PR, like you originally did, to document it in the change log.

[fsmanuel]

Up until now you've had those embroider tests deactivated, not sure why, but my PR reactivates them because they are necessary moving to Ember 5.

I activated the scenarios to see what is failing.

With ember-data 4.8 (current master) and the embroider safe scenario we get:

 ERROR in ../../node_modules/@ember-data/model/-private/record-state.js 45:0-76
Module not found: Error: Can't resolve '@ember-data/tracking/-private' in '$TMPDIR/embroider/62ed3b/node_modules/@ember-data/model/-private'
 @ ./assets/dummy.js 563:13-85

ERROR in ../../node_modules/@ember-data/store/-private/managers/record-array-manager.js 4:0-65
Module not found: Error: Can't resolve '@ember-data/tracking/-private' in '$TMPDIR/embroider/62ed3b/node_modules/@ember-data/store/-private/managers'
 @ ./assets/dummy.js 710:13-102

ERROR in ../../node_modules/@ember-data/store/-private/record-arrays/identifier-array.js 50:0-76
Module not found: Error: Can't resolve '@ember-data/tracking/-private' in '$TMPDIR/embroider/62ed3b/node_modules/@ember-data/store/-private/record-arrays'
 @ ./assets/dummy.js 743:13-103

With ember-data 4.11 and the embroider safe scenario we get:

Error: Assertion Failed: EmberObject.create no longer supports defining computed properties. Define computed properties using extend() or reopen() before calling create().

So that seems connected to the decorator topic and will hopefully fixed by your fix. I totally agree with you, that we want them activated for a ember 5 compatible release.

Please note that some of my refactoring is needed for the embroider scenarios to pass.
You will probably face the same issues if you try to run those embroider tests on that other PR you created.

Yeah, lets see what needs to be done to get them green for #736 or decide to leave it to your PR to get it working. It's not needed for the current release cycle because we did not support it in the past.

[MelSumner]

@Pixelik looks like there are some conflicts to be resolved, can you work on those when you get a chance? TYSM!

[runspired]

This check will not work in EmberData 5.0+

[runspired]

content is private and not stable. the other option of toArray is also not valid. Just treat EmberData things as native arrays, unless its a PromiseManyArray in which case you should error. PromiseManyArray can be detected in 5.0+ by something being a thenable and not having usual array methods but having a forEach method.

[runspired]

Requires updates to support EmberData 5.x.

Uncertain if support for 4.x is also desired ... but the current approach does not adequately support 4.x either.