Drop Recommended Rules for Deprecated Ember 3.x Features

elwayman02 commented 1 year ago

Proposal

Since Ember 3 is currently unsupported, we should remove rules from the recommended config that only existed to help clean up deprecated features which were removed in Ember 4.0. We could add a secondary "legacy" config containing these deprecation rules that legacy apps could install if they have not reached Ember 4 yet, and this may prove to be a useful long-term paradigm to allow progressive enhancement/cleanup of the linter while maintaining a backwards-compatible config for legacy apps. Either way, we should not keep validating code against these rules when violations of the lint rules would break in supported versions of Ember anyway.

Why is this important?

Requiring fewer rules to be checked is great for linter performance, and having an updated, relevant recommended config maintains high confidence in the package from consumers. In some cases, these rules may even cause issues with modern Ember code as our syntax changes in ways we couldn't predict when using those legacy features.

Which rules for deprecated features should be removed?

[ ] ember/no-deeply-nested-dependent-keys-with-each
[ ] ember/no-volatile-computed-properties
[ ] ember/closure-actions
[ ] ember/no-function-prototype-extensions
[ ] ember/no-old-shims
[ ] ember/no-string-prototype-extensions
[ ] ember/no-get-with-default
[ ] ember/no-try-invoke
[ ] ember/no-test-module-for (I don't believe the version of ember-qunit required for Ember 4 still contains these deprecated methods?)

Feel free to correct if any of the above is still supported in Ember 4, or if I've missed a rule that should be here.

How do we roll this out?

I'd argue removing rules isn't necessarily a breaking change, though if the team feels strongly that it should be a major release, I'm not opposed to it. We should establish a config containing these rules which can be adopted by apps that still need them, at the same time we remove them from the recommended config. Appropriate documentation should be added to establish how and when to use the legacy config in conjunction with the recommended config. I believe there should never be a rule that exists in both configs at the same time, by definition. Either it's a rule we need for at least one supported version of Ember, or it's a bridge rule that we have for legacy reasons.

Open Questions

Some of the rules may actually be holdovers from Ember 1 or 2, and perhaps shouldn't be part of the legacy config either. I would recommend defining the legacy config as containing rules for 1 major version prior to the oldest supported LTS, and anything from even older major versions should simply be available on an opt-in basis only.

elwayman02 commented 1 year ago

For visibility and cross-referencing decisions on how to move forward consistently, I've filed a similar issue for ember-template-lint: https://github.com/ember-template-lint/ember-template-lint/issues/2976

bmish commented 1 year ago

Thanks for the proposal. I generally don't agree with removing rules simply for targeting older versions, as I've described in my full response https://github.com/ember-template-lint/ember-template-lint/issues/2976#issuecomment-1716617693.

ember-cli / eslint-plugin-ember