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

Add eslint-plugin-erb to provide linting of our .js.erb files #5549

Closed

Conversation

gravitystorm
Copy link
Collaborator

Fixes #5523

This is a draft, since our .js.erb files don't currently pass the linting. I've fixed the straightforward ones in #5548 but there are a few more to complete before this can be merged.

I've opened this pull request to make it easier for other developers (who have more JS skills) to help. My recommendation is to checkout this pull request locally, fix the errors, and submit those fixes separately as PRs (e.g. cherry-pick your fixes onto our master branch).

…b files

These have been flagged by eslint-plugin-erb, and need to be fixed before
that plugin can be enabled.
@gravitystorm gravitystorm added the work-in-progress Pull request is not ready to be merged label Jan 24, 2025
@mmd-osm
Copy link
Contributor

mmd-osm commented Jan 24, 2025

✖ 40 problems (31 errors, 9 warnings)
26 errors and 0 warnings potentially fixable with the --fix option.

Is --fix of any help here, have you tried this out already?

@gravitystorm
Copy link
Collaborator Author

Is --fix of any help here, have you tried this out already?

We run eslint via bundle exec rails eslint, and that doesn't support the --fix option. Our configuration doesn't support running eslint directly, since we use ruby code in the configuration.

I'd like to get it running directly at some point, to improve integration between eslint and IDEs, which are more likely to support eslint directly than via rails tasks. Maybe there's also a way to get the --fix working via rails.

@mmd-osm
Copy link
Contributor

mmd-osm commented Jan 24, 2025

According to https://github.com/openstreetmap/openstreetmap-website/blob/master/lib/tasks/eslint.rake#L23 we would need to use bundle exec rails eslint:fix instead. This seems to work for me at least.

It's not documented anywhere unfortunately. Would be a good fit for https://github.com/openstreetmap/openstreetmap-website/blob/master/CONTRIBUTING.md#coding-style

Situation before: (file: /app/assets/javascripts/osm.js.erb)


    9:9   error  Extra space before value for key 'MATOMO'                                                   key-spacing
   12:19  error  Extra space before value for key 'MAX_REQUEST_AREA'                                         key-spacing
   13:18  error  Extra space before value for key 'SERVER_PROTOCOL'                                          key-spacing
   14:13  error  Extra space before value for key 'SERVER_URL'                                               key-spacing
   15:14  error  Extra space before value for key 'API_VERSION'                                              key-spacing
   16:9   error  Extra space before value for key 'STATUS'                                                   key-spacing
   17:24  error  Extra space before value for key 'MAX_NOTE_REQUEST_AREA'                                    key-spacing
   18:15  error  Extra space before value for key 'OVERPASS_URL'                                             key-spacing
   19:23  error  Extra space before value for key 'OVERPASS_CREDENTIALS'                                     key-spacing
   20:16  error  Extra space before value for key 'NOMINATIM_URL'                                            key-spacing
   21:18  error  Extra space before value for key 'GRAPHHOPPER_URL'                                          key-spacing
   22:19  error  Extra space before value for key 'FOSSGIS_OSRM_URL'                                         key-spacing
   23:23  error  Extra space before value for key 'FOSSGIS_VALHALLA_URL'                                     key-spacing
   24:17  error  Extra space before value for key 'DEFAULT_LOCALE'                                           key-spacing
   27:20  error  Extra space before value for key 'THUNDERFOREST_KEY'                                        key-spacing
   31:18  error  Extra space before value for key 'TRACESTRACK_KEY'                                          key-spacing
   34:20  error  Extra space before value for key 'LAYER_DEFINITIONS'                                        key-spacing
   35:22  error  Extra space before value for key 'LAYERS_WITH_MAP_KEY'                                      key-spacing
   37:15  error  Extra space before value for key 'MARKER_GREEN'                                             key-spacing
   38:13  error  Extra space before value for key 'MARKER_RED'                                               key-spacing
   40:14  error  Extra space before value for key 'MARKER_ICON'                                              key-spacing
   41:17  error  Extra space before value for key 'MARKER_ICON_2X'                                           key-spacing
   42:16  error  Extra space before value for key 'MARKER_SHADOW'                                            key-spacing
   44:18  error  Extra space before value for key 'NEW_NOTE_MARKER'                                          key-spacing
   45:19  error  Extra space before value for key 'OPEN_NOTE_MARKER'                                         key-spacing
   46:21  error  Extra space before value for key 'CLOSED_NOTE_MARKER'                                       key-spacing
   64:5   error  Assignment to function parameter 'search'                                                   no-param-reassign
   83:54  error  'match' is defined but never used                                                           no-unused-vars
  161:5   error  Assignment to function parameter 'hash'                                                     no-param-reassign
  231:44  error  Unexpected mix of '+' and '*'. Use parentheses to clarify the intended order of operations  no-mixed-operators
  232:41  error  Unexpected mix of '+' and '*'. Use parentheses to clarify the intended order of operations  no-mixed-operators

After running eslint:fix:

   64:5   error  Assignment to function parameter 'search'                                                   no-param-reassign
   83:54  error  'match' is defined but never used                                                           no-unused-vars
  161:5   error  Assignment to function parameter 'hash'                                                     no-param-reassign
  231:44  error  Unexpected mix of '+' and '*'. Use parentheses to clarify the intended order of operations  no-mixed-operators
  232:41  error  Unexpected mix of '+' and '*'. Use parentheses to clarify the intended order of operations  no-mixed-operators

@hlfan
Copy link
Contributor

hlfan commented Jan 24, 2025

64:5 error Assignment to function parameter 'search' no-param-reassign is really unnecessary, the entire function should be refactored to use the URLSearchParams Web API.

@gravitystorm
Copy link
Collaborator Author

Superseded by #5559

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-in-progress Pull request is not ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add linting for .js.erb files
3 participants