-
Notifications
You must be signed in to change notification settings - Fork 27
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
Upgrade ESLint and Babel. Use Wren Security ESLint conventions. #165
Conversation
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. |
1ff59c1
to
df7a484
Compare
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. |
I see. Would it be OK to handle that in a separate PR? |
c30abc4
to
b77a465
Compare
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. |
553499c
to
0fef4d2
Compare
0fef4d2
to
697a724
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, ty.
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.