search

WrenSecurity / wrenam

Community fork of OpenAM, an authentication and authorization system originally developed by ForgeRock.
Other
43 stars 27 forks source link

Upgrade ESLint and Babel. Use Wren Security ESLint conventions. #165

Closed krausvo1 closed 9 months ago

krausvo1 commented 9 months ago

This PR adds our ESLint conventions (@wrensecurity/eslint-config) to the project. ESLint, Babel and related Grunt dependencies have been upgraded to the latest versions.

React specific ESLint rules come from ForgeRock's former forgerock/react shared config. Those could be moved to a separate shared config in the future.

The ESLint rules I have deleted are already present in @wrensecurity/eslint-config.

I have tested both admin and user UI, everything works fine. Production build runs without any errors. Built code sync in development mode also works as expected.

pavelhoral commented 9 months ago

On a second thought all of non-ES5 features have to be dropped because of the overhead that the transpilation will introduce (helper functions in every). We can switch to ES2022+ when we drop RequireJS (as r.js understands only ES5). This will be hopefully done soon, but not right now.

krausvo1 commented 9 months ago

There is already lot of non-ES5 features in current code (and the code is currently being transpiled). Do you think we should get rid of these already present occurences? If so, I think we could do it in a separate PR and keep this PR aimed at adopting new shared ESLint configuration.

I have dropped non-ES5 changes my changes have introduced. Most of the changes are fixes of ESLint errors related to formatting.

pavelhoral commented 9 months ago

There is already lot of non-ES5 features in current code (and the code is currently being transpiled). Do you think we should get rid of these already present occurences? If so, I think we could do it in a separate PR and keep this PR aimed at adopting new shared ESLint configuration.

I have dropped non-ES5 changes my changes have introduced. Most of the changes are fixes of ESLint errors related to formatting.

I am OK with sending all code through some sort of limited transpilation (or even the full one) if it does not generate additional unwanted code for ES5-only files.

krausvo1 commented 9 months ago

I see. Would it be OK to handle that in a separate PR?

krausvo1 commented 9 months ago

I have added our shared ESLint React config, added function parameters related rules (inspired by Wren:IDM rules) and dropped copyright headers updates from files with only trivial whitespace changes.