Add eslint with prettier plugin and integrate it in preversion/release pipeline

[IvanPostu]

Before submitting...

• I have searched for duplicate or closed issues.
• I have read the CONTRIBUTING document and my issue is following the guidelines.
• I have read the template completely before filling it in.

Context

It would be great to have a unique code style and formating and in order to achieve this we could integrate esling with prettier plugin and make it as a part of preversion/release pipeline.

Also it could be as a part of github action which runs tests.

Current Behavior

No response

Expected behavior

No response

Possible Solutions or Causes

No response

Your Environment

• Version used:
• Browser Name and version:
• Operating System and version (desktop or mobile):
• Additional information you want to tell us:

[wuda-io]

why did you split it in 3 parts?
it is a bit difficult to review. Should they be merged in order?

[IvanPostu]

I aimed to divide into 3 separate PRs that should be merged into each other for better granularity, but because I cannot create branches within the repository, the base branch of PR can be either master or v2-dev.

[IvanPostu]

I can explain them briefly:

• enhancement-506: Add eslint with prettier plugin and integrate it in preversion/release pipeline (Part 1 - config) #507 - here I introduce eslint with prettier and mark all existing problems to be treated as warnings (in this case npm run lint will finish execution with the return code 0)
• enhancement-506-2: Add eslint with prettier plugin and integrate it in preversion/release pipeline (Part 2 - fix all prettier issues and increase severity) #508 - here I refactor the whole project to follow prettier rules and increase prettier severity to error which means if at least one rule is violated, then npm run lint will finish execution with a return code different from 0
• enhancement-506-3: Add eslint with prettier plugin and integrate it in preversion/release pipeline (Part 3 - fix some eslint errors and increase severity) #509 - here I increase severity for a few eslint rules to error and refactor the project to follow these rules

[gselderslaghs]

Hi @IvanPostu

I've reviewed your changes and can agree upon your request implementing typescript eslint as it would be a great enhancement improving coding-standards within the current codebase, new implementations and feature changes

Regarding implementing the prettier plugin and reformatting the current codebase I'm not convinced since of the size of the project it might be a breaking change as it would be exceptionally difficult to review
Would it be an idea to implement the prettier plugin in a separate linter configuration that can be used to review new implementations and feature changes, thus giving insights on improving the code quality on those subjects?

If typescript linting would be implemented, hardening the configuration declarations from warning to error in order to make sure implementations and feature changes follow the coding standards. This configuration hardening would need further investigation and clarification what standards should be applied, what are your thoughts about this? Which of the current options do you see as necessary to comply with coding standards?

[wuda-io]

The problem is that there are too many changes at once. Also in the meantime there are other PRs which want to be merged. So there are many conflicts and it is very hard to keep track. Maybe we should aim for a partly config and adapt step by step. Is this possible with less effort?

Also in the future we should split some of the library into better manageable parts (e.g. colors). I think the submodules are a good choice here as @danice already demonstrated with the docs.

For now, I can not merge this.

[gselderslaghs]

Created a new PR #534 that only introduces config changes, removed the prettier plugin for now and changed eslint config with hardening in order to produce errors instead of warnings so future builds would require to hold on to the coding standards

[gselderslaghs]

I've been reviewing the latest changes added by @wuda-io, as mentioned in my review on v2.2.1 the latest changes, eg. changing the M_(Component) object to array, introduces backward compatibility issues

Excerpt from version review

Referencing to #560, I notice that in this commit we changed the M_ from object orientated to array based
This introduces backwards compatibility issues on v2.2.1 for applications depending on the object orientated approach
For now, my suggestion would be to revert changing the M_ from object orientated to array based back to object orientated approach and change @typescript-eslint/no-explicit-any to warning instead of error in eslint to let the build pass

In meanwhile, we can look for alternative object type declaration to suppress the warnings in one of the next releases

As of v2.2.1 upgrading from v2.1.1 to v2.2.1 is not working or would introduce a major upgrade in existing codebases, which makes it more of a major upgrade instead of minor and would require to introduce additional documentation for the upgrading process to end-users

For the maintainability and scalability regarding Materialize I know it's important to end-users to upgrade with less effort as possible without having to fix broken components, propose to revert, WDYT?

I've been delving into this issue lately, the most fine grained approach to introduce the revert, is to create a new version branch from v2.2.0, then introduce the #560 pull request, without changing the M_(Component) from object orientated to array, but changing the eslint @typescript-eslint/no-explicit-any config from error to warning

[wuda-io]

What do you mean from object based to array based? I think we should not change from error to warning.
Can you give me an example where it breaks?

I think we should work with arrays by default and just always parse inputs into array format. If there is an object we convert it to an array with length 1. e.g. {} => [{}], [1,2] => [1,2], 3 => [3]. But I didn't understand where you mean. The init function?

[gselderslaghs]

I made the review on commit #563
The problem here might be when components are initialized by javascript (shadow DOM for example), the components where accessible by HTMLElement.C_ComponentName in versions < 2.2.1, since 2.2.1 now we have to use HTMLElement.['C_ComponentName']

I don't see any advantage of using an array as type, since the reference is basically the object itself, it might be confusing and as I mentioned it would require existing applications depending on object component call by html element to change the HTMLElement.C_ComponentName calls in the entire codebase to HTMLElement.['C_ComponentName']

The most fine grained approach would be to define correct type or otherwise create an ignore path in the eslint for the any types on HTMLElement.C_ComponentName as does not seem logical changing to array while the content of the array is an object, I also found this as an possibility during my refactoring to apply escoding standard to pass the linter, but did not apply it in the codebase for the previous mentioned reason by keeping in mind end-users do not run into errors and would require to update their codebases as wel after updating to the latest versions

WDYT, should I try to find a solution for my concerns?