feat(rome_js_analyze): `noParameterProperties`
Summary
This implements ESLint's no-parameter-properties.
Test Plan
Unit tests and doc-tests included.
Deploy Preview for docs-rometools ready!
Built without sensitive environment variables
| Name | Link |
|---|---|
| Latest commit | 14d9ebec55a80df691de6a6bc09137f911482889 |
| Latest deploy log | https://app.netlify.com/sites/docs-rometools/deploys/63eaa9afff494b00084858f6 |
| Deploy Preview | https://deploy-preview-3874--docs-rometools.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
The documentation says that this rule is deprecated
The documentation says that this rule is deprecated
Yes, the rule was replaced by parameter-properties.
However, this does not fit with the philosophy of Rome (no or use, and do not provide configuration when it is not necessary).
By the way, I tried to implement an action to turn the parameter property into a regular property.
However, I encountered an issue: when I mutate a node, I cannot mutate a descendant without invalidating the previous mutation. It is particularly inconvenient.
I have to figure out how to overpass this limitation.
I have some reservations about this rule, here's my concerns:
- if and once this rule is promoted, which group would this rule fit?
- there are frameworks that use these properties, such as NestJS: https://docs.nestjs.com/providers#services, wouldn't this rule create friction?
- are there any other motivations other than the the confusing part? I used parameter properties inside constructors many times, and I fail to see this rule in the Rome grand scheme. Would you help me to understand this rule?
if and once this rule is promoted, which group would this rule fit?
I think that Style could be a good fit.
there are frameworks that use these properties, such as NestJS
I was not aware of this use. It could definitively bring friction.
are there any other motivations other than the the confusing part?
The arguments I could add:
- TypeScript should avoid code generation. Parameter properties are syntactic sugars. Yes, TypeScript has other features that generate code (enums, namespaces). However, namespaces are no longer recommended, and enums are popular and could even be one day part of JavaScript.
- It is not possible to declare a private parameter property (using she-bang #).
By the way, I tried to implement an action to turn the parameter property into a regular property.
However, I encountered an issue: when I mutate a node, I cannot mutate a descendant without invalidating the previous mutation. It is particularly inconvenient.
I have to figure out how to overpass this limitation.
I think the easiest way to implement this without encountering conflicts in the mutation would be to:
- Remove the
TsPropertyParameterfrom its parentJsConstructorParameterListusingsplice_slots - Replace the old constructor parameter list with the modified list in the
JsClassMemberListcontaining the constructor usingreplace_child - Insert the new
JsPropertyClassMembernode in the modified class member list usingsplice_slots - Queue an operation to replace the old class member list with the modified list in the
BatchMutationby callingreplace_node
This PR is stale because it has been open 14 days with no activity.
@Conaclos Do you have a version with the mutation problem so I can look it up?
I think we may have a version of BatchMutation that fix this, but it was never merged because it is slower.
@xunilrj I committed my last wip commit. I did not take a look to your proposal yet.
@xunilrj I think that the action could be made in a separate PR. I could like to propose the PR in its current state.