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

Drop Recommended Rules for Deprecated Ember 3.x Features #2976

Open
11 tasks
elwayman02 opened this issue Sep 12, 2023 · 4 comments
Open
11 tasks

Drop Recommended Rules for Deprecated Ember 3.x Features #2976

elwayman02 opened this issue Sep 12, 2023 · 4 comments
Labels

Comments

@elwayman02
Copy link
Contributor

elwayman02 commented Sep 12, 2023

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?

In some cases, these rules may even cause issues with modern Ember code. For example, the no-implicit-this rule throws errors when using Template Imports to reference component classes via the component helper (e.g. someYieldedValue=(component FooComponent)). Instead of forcing modern Ember apps to manually disable these rules which are guarding against features that no longer exist in supported versions of Ember, we should stop recommending the rules by default.

Which rules for deprecated features should be removed?

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 I mentioned above are actually 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
Copy link
Contributor Author

For visibility and cross-referencing decisions on how to move forward consistently, I've filed a similar issue for eslint-plugin-ember: ember-cli/eslint-plugin-ember#1950

@bmish
Copy link
Member

bmish commented Sep 12, 2023

Thanks for the proposal.

I generally do not agree with removing rules from the recommended config just because they are targeting old Ember versions.

I'd prefer that we keep rules in the recommended config unless they are incompatible with modern Ember and it's not possible to make them compatible. Oftentimes, we can update a rule with a simple fix or heuristic in order to ensure cross-version compatibility. But in my experience, incompatibilities are rare, and most rules targeting old Ember versions do not cause any issues.

In addition, needlessly removing rules from the config causes churn when there's little or no cost to continuing to include them. Something that does have a cost (in terms of maintenance burden and reduced ease-of-use) is adding additional configs.

And while we don't technically need to support users on Ember 3 anymore, I'm happy to continue supporting them if the cost is minimal. Especially when most of these older rules are simply about detecting deprecated patterns that aren't supported anymore but could easily pop-up when copy-pasting in code from an old article or codebase or receiving a suggestion from an AI copilot, etc.

So what I would recommend is:

  1. Identify the complete list of recommended rules that are incompatible with modern Ember. I only see no-implicit-this mentioned regarding this.
  2. Determine if incompatible rules can be made compatible (e.g. become a no-op if they aren't relevant anymore).
  3. If there are truly incompatible rules that cannot be updated to co-exist across Ember versions, remove these from the recommended config as a breaking change in a major release.

@bmish bmish added the breaking label Sep 12, 2023
@bertdeblock
Copy link
Collaborator

bertdeblock commented Sep 13, 2023

FYI, the unbound helper was never deprecated nor removed, so I think the no-unbound rule is still relevant.

@elwayman02
Copy link
Contributor Author

FYI, the unbound helper was never deprecated nor removed, so I think the no-unbound rule is still relevant.

Good catch. I saw it was marked deprecated in Ember 1.x, but I guess they never got around to removing it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants