refactor: safely enforce ESLint rules on non-conflicting files

[harshgupta2125]

Closes #12260

refactor: Enforces ESLint rules for semicolons, function spacing, and comma spacing across the codebase to prevent noisy formatting diffs in future PRs.

Technical

To prevent huge merge conflicts for contributors currently working on open PRs, I executed this rollout safely:

• Enable semi, space-before-functiion-paren, and comma-spacing in .eslintrc.json.
• Authored a Python script utilizing the GitHub API to identify all files currently touched by open PRs.
• Ran eslint --fix strictly on files with no active PRs (skipping the locked files).
• Remaining conflicting files can be safely linted in a future sweep once the current queue of PRs is merged.

Testing

• Verified that formatting changes are strictly stylistic.
• Ran npm run test:js locally -> 302/302 tests passed.

Screenshot

NA

Stakeholders

@RayBB

[Copilot]

Pull request overview

This PR standardizes JavaScript formatting to reduce future diff noise by enabling stricter ESLint rules and applying auto-fixes across a large set of non-conflicting files.

Changes:

• Enable ESLint rules for semicolons, function-paren spacing, and comma spacing.
• Apply ESLint auto-fixes across many JS/Vue/source + test files to conform to the new rules.
• Update various unit tests and sample HTML fixtures to match the enforced formatting.

Reviewed changes

Copilot reviewed 132 out of 178 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/unit/js/utils.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/signup.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/service-worker-matchers.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/SelectionManager.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/SearchBar.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/search.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/sample-html/utils-test-data.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/sample-html/lists-test-data.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/sample-html/dropper-test-data.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/sample-html/checkIns-test-data.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/python.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/jsdef.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/jquery.repeat.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/idValidation.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/html-test-data.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/editionsEditPage.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/Browser.test.js	Formatting updates to comply with new ESLint rules.
tests/unit/js/autocomplete.test.js	Formatting updates to comply with new ESLint rules.
stories/Button.stories.js	Formatting updates to comply with new ESLint rules.
static/bookmarklets/import_webbook.js	Formatting update to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/waitlist.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/utils.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/type_changer.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/Toast.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/template.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/team.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/tabs.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/stats/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/star-ratings/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/service-worker.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/service-worker-matchers.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/SearchUtils.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/SearchPage.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/search.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/return-form/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/readinglog_stats.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/python.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/private-button.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/patron_exports.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/partner_ol_lib.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/ol.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/ol.analytics.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/offline-banner.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/nonjquery_utils.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/native-dialog/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/my-books/store/index.js	Formatting updates; touched code includes MyBooks store initialization.
openlibrary/plugins/openlibrary/js/my-books/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/my-books/CreateListForm.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/merge-request-table/MergeRequestTable/TableHeader.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/merge-request-table/MergeRequestTable.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/merge-request-table/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/lists/ListViewBody.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/lists/ListService.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/list_books.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/librarian-dashboard/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/lazy-thing-preview.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/lazy-carousel.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/jsdef.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/jquery.repeat.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/isbnOverride.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/interstitial.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/ile/utils/ol.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/idValidation.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/ia_thirdparty_logins.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/graphs/plot.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/graphs/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/go-back-links.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/fulltext-search-suggestion.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/following.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/editions-table/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/edition-nav-bar/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/edition-nav-bar/EditionNavBar.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/dropper/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/dropper/Dropper.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/covers.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/compact-title/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/clampers.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/bulk-tagger/models/Tag.js	Formatting updates; touched code includes tag sorting logic.
openlibrary/plugins/openlibrary/js/bulk-tagger/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/bulk-tagger/BulkTagger/SortedMenuOptionContainer.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/bulk-tagger/BulkTagger/MenuOption.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/Browser.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/breadcrumb_select/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/book-page-lists.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/banner/index.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/affiliate-links.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/admin.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/add-book.js	Formatting updates to comply with new ESLint rules.
openlibrary/plugins/openlibrary/js/add_provider.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/ObservationForm/Utils.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/ObservationForm/ObservationService.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/ObservationForm/components/ValueCard.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/ObservationForm/components/SavedTags.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/ObservationForm/components/OLChip.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/ObservationForm/components/CategorySelector.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/ObservationForm/components/CardHeader.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/ObservationForm/components/CardBody.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/ObservationForm.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/MergeUI/TextDiff.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/MergeUI/MergeRowJointField.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/MergeUI/MergeRowField.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/MergeUI/MergeRow.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/MergeUI/ExcerptsTable.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/MergeUI/EditionSnippet.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/MergeUI/AuthorRoleTable.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/MergeUI.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/lit/OLReadMore.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/lit/OlPagination.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/lit/OLChipGroup.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/lit/OLChip.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/utils/lcc.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/utils.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/ShelfProgressBar.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/ShelfLabel.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/ShelfIndex.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/Shelf.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/OLCarousel.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/FlatBookCover.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/DemoA.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/CSSBox.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/ClassSlider.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/BooksCarousel.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/BookRoom.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer/components/BookCover3D.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/LibraryExplorer.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/IdentifiersInput/utils/utils.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/HelloWorld.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/dev/vite.config.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/BulkSearch/utils/searchUtils.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/BulkSearch/utils/samples.js	Formatting updates to comply with new ESLint rules.
openlibrary/components/BulkSearch/components/NoBookCard.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/BulkSearch/components/MatchTable.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/BulkSearch/components/MatchRow.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/BulkSearch/components/BookCard.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/BulkSearch.vue	Formatting updates to comply with new ESLint rules.
openlibrary/components/BarcodeScanner/utils/classes.js	Formatting updates to comply with new ESLint rules.
.eslintrc.json	Adds new ESLint enforcement rules for semicolons and spacing.

[Copilot]

MyBooksStore initializes _store with userkey, but the getter/setter use this._store.userKey. This means getUserKey() will always return undefined (and setUserKey() creates a new property), breaking consumers. Rename the stored field to userKey (or update the accessor methods) so the key is consistently stored/read.

[Copilot]

In compare(), the second tag type comparison uses lowerB.tagtype (lowercase t) instead of lowerB.tagType. Since createComparableTag() only returns tagType, this condition will never behave correctly and can produce incorrect sort ordering. Use lowerB.tagType here.

[Copilot]

Because lint:js runs eslint --ext js,vue . across the whole repo (see package.json), enabling these rules as errors (semi, space-before-function-paren, comma-spacing) will immediately break linting for files that weren’t auto-fixed in this staged sweep. For example, openlibrary/components/IdentifiersInput.vue still has idConfigs: function() { and many statements without semicolons, which violate the new config. Either fix the remaining violations before landing this, or add temporary overrides/ignorePatterns so CI isn’t broken while the rollout is incremental.

Suggested change

	"semi": ["error", "always"],
	"space-before-function-paren": ["error", "always"],
	"comma-spacing": ["error", { "before": false, "after": true }]
	"semi": ["warn", "always"],
	"space-before-function-paren": ["warn", "always"],
	"comma-spacing": ["warn", { "before": false, "after": true }]

[RayBB]

1. Please provide the script you used to get this list of js files open in PRs right now. Also provide the list of files you got from running it.
2. We need to make sure we exclude those files from the rule in the config otherwise they will automatically be fixed by pre-commit (like the seemed to have been in the most recent commit).
3. Separately, since it looks like we'll be touching a ton of files with this, maybe you could investigate what it would look like to use biome instead. What would the pros and cons be? It's a modern opinionated formatter.

[harshgupta2125]

Thanks for the review! Yes, you are totally right, the pre-commit bot picked up those other files because the config applied everywhere.

1. The Script and Files
   Here is the gist: https://gist.github.com/harshgupta2125/179fdfc28f3a9e15e1e172c8709acc43
   It has the python script I wrote, and the clean list of the 78 .js and .vue files that are currently locked in other open PRs.

2. About Biome
   I looked into it and Biome looks awesome.
   It is super fast (written in Rust) and it's nice that it handles both formatting and linting in one tool.
   But the main issue I found is that its support for .vue files is still experimental right now.
   Since our frontend uses a lot of Vue components, I think switching to Biome right now might cause some unpredictable parsing issues for us.
   well its a good approach for new projects. but in future whenever they released a stable version for .vue files i would like to make a propsal for migrating to biome.

I think it is safer to stick with ESLint for now until Biome's Vue support becomes fully stable. What do you think?

If that sounds good to you, I can just add these 78 files to .eslintignore and push the commit to stop the bot from formatting them. Let me know!

[RayBB]

@harshgupta2125 can you update your script to open look at PRs that aren't drafts? We want to focus on PRs that are actively ready for review. How many files is it then?

Also, perhaps we could use biome for just the non-vue files (which are the majority) and stick with eslint (and these rules) for Vue files. Would you be up for doing a small comparison and describe what it would take to switch to Biome with minimal disruption? Also could you check how much faster biome is to run vs our current tool (for our codebase)?

[harshgupta2125]

Thanks @RayBB! I updated the script to skip Draft PRs—the list of locked files dropped from 78 down to 46.

I ran the speed tests and the results is :

• ESLint: ~18.6s
• Biome: ~14.7s

Well i am using an older hardware device on it, Biome is already about 20% faster than ESLint. On a modern dev machine or the GitHub CI servers, it would probably be lightning fast.

What it would take to switch:

To do this with minimal disruption, we could use a "hybrid" setup:

• Biome: Handle all .js files (the majority of the repo).
• ESLint: Keep it strictly for .vue files since Biome's Vue support is still experimental.
• Config: We'd need to create a biome.json that matches our current rules and update package.json to run both tools.

My suggestion:

Since we have to keep ESLint for Vue anyway, I think we should stick with this ESLint PR for now to get the codebase standardized immediately. Then, I’d be happy to open a follow-up PR for a "Hybrid" migration (using Biome for JS and ESLint for Vue). This is actually what companies like Discord, Microsoft, and Vercel do to balance speed with specific framework needs.

What do you think? If we are sticking with ESLint for now, I'll update the .eslintignore with those 46 files and push!

[tfmorris]

If you're going to make a massive code format change, I'd prioritize doing it in a single commit so that it's easy to add to .git-blame-ignore-revs file. https://akrabat.com/ignoring-revisions-with-git-blame/ If you want to minimize impact on code in-process, that can be done by communicating with developers to find a good quiescent time to phase in the change, but merge conflicts are a fact of life in any active project.

…

Message ID: ***@***.*** com>

[RayBB]

@tfmorris that's a good suggestions about git-blame-ignore-revs.

The staff have also been pretty clear that it's a very high priority to avoid doing anything that creates more work for PR review (like merge conflicts) so we should definitely to minimize the commits.

So for @harshgupta2125

1. We do want to ignore those 46 file but only for these specific rules? If that's not possible I'll need to check in with the team on how they want to move forward.
2. If you're up for it, could you also create a draft PR to turn on move from eslint to biome for just the JS files (trying to keep similar rules if possible)? I'd like to run this option by the team since it seems likely to be a viable long term solution. I think if we are gonna touch a lot of JS files it's better to just do it once than twice.

[harshgupta2125]

Thanks @tfmorris and @RayBB! I just learned about git-blame-ignore-revs—that makes total sense for a change this much size of files. I’ll make sure the final version is a single and clean commit to keep the history tidy.

I'm really excited to try the hybrid approach. I'll start working on a separate Draft PR now to move the JS files to Biome while keeping ESLint for the Vue components.

I’ll link the new Draft PR here as soon as I have the basic setup and speed comparisons ready!