Enable lint rule `require-atomic-updates`

[samreid]

From https://github.com/phetsims/chipper/issues/814#issuecomment-838724968 we agreed to enable the lint rule "require-atomic-updates", but there are 6 offenses at the moment, currently all in rosetta. They should be fixed or have the lint rule disabled for each occurrence, and the rule in chipper should be turned on. @jbphet can you please take a look?

[jbphet]

@liam-mulhall - I'm turning this over to you to have a look. We can discuss in our next weekly meeting if it would help.

[liammulh]

@jbphet, can we discuss this at next week's meeting?

[liammulh]

I'll enable this rule in Rosetta 2.0, but I'm not going to enable it in old Rosetta.

[liammulh]

Done in https://github.com/phetsims/rosetta/commit/0e21ccd0ff26c46a5ae3af0fc3d2b11688b4e3fd.

[liammulh]

I need to do this again on the liam_fork branch.

[liammulh]

Done (again) in https://github.com/phetsims/rosetta/commit/2bba9da83ce3d2fc61c29ea8a4c573ef20a904fe for the new branch.

[samreid]

As discovered in https://github.com/phetsims/chipper/issues/946, there are TODOs in the code referring to this issue. Reopening.

[liammulh]

I don't see any TODOs referencing this issue:

➜  rosetta git:(master) pwd
/Users/liam/Stuff/repos/phet/rosetta
➜  rosetta git:(master) rg "TODO"
js/rosetta.js
4: * TODO: Relocate code in this file that doesn't have to do with configuring the app, setting up routes, etc. See https://github.com/phetsims/rosetta/issues/190#issuecomment-682169944.
75:// TODO: Move this to a more appropriate location. See https://github.com/phetsims/rosetta/issues/190#issuecomment-682169944.
76:// TODO: Determine if setting a variable for the config is preferable to this approach. See https://github.com/phetsims/rosetta/issues/190#issuecomment-682169944.
84:// TODO: Determine if it's possible to get the config before setting up the logger. (So we don't have to update it.) See https://github.com/phetsims/rosetta/issues/190#issuecomment-682169944.
89:// TODO: Move this into its own file. See https://github.com/phetsims/rosetta/issues/190#issuecomment-682169944.

js/renderLocaleStringReport.js
38://     // TODO: There should be some way to figure this out based on the metadata.

js/routeHandlers.js
180:  // TODO: Create a separate file that contains code to get the saved strings. Then simply call that code here. See https://github.com/phetsims/rosetta/issues/190#issuecomment-682169944.

js/getRosettaConfig.js
4: * TODO: Relocate code in this file that doesn't have to do with getting the Rosetta configuration file. See https://github.com/phetsims/rosetta/issues/190#issuecomment-682169944.
144: * // TODO: Put this function in a more appropriate file. See https://github.com/phetsims/rosetta/issues/190#issuecomment-682169944.
➜  rosetta git:(master) 

I don't see a link to this issue:

➜  rosetta git:(master) rg "https://github.com/phetsims/chipper/issues/1382"
➜  rosetta git:(master) 

I don't see any TODOs or links to this issue in Rosetta 2.0:

➜  rosetta git:(rosetta_2.0_rev2) rg "TODO"
➜  rosetta git:(rosetta_2.0_rev2) rg "https://github.com/phetsims/chipper/issues/1382"
➜  rosetta git:(rosetta_2.0_rev2) 

[samreid]

I believe this is referring to

https://github.com/phetsims/chipper/blob/38136f078626fc6a3dea1f8656d0f33fa01e8948/eslint/.eslintrc.js#L716-L717.

Can we now enable that rule?

[liammulh]

@samreid, this lint rule is now enabled on the master branch of Rosetta.

[liammulh]

https://github.com/phetsims/rosetta/blob/35bb0b70d5586bf246c24dc23faa78dce8122f8b/.eslintrc.cjs#L31-L34.

[samreid]

We currently have 15 failures for this lint rule, so it cannot be enabled without code changes:

All results (repeated from above)

/Users/samreid/apache-document-root/main/aqua/js/server/QuickServer.js
  238:13  error  Possible race condition: `lastBroken` might be reassigned based on an outdated value of `lastBroken`  require-atomic-updates

/Users/samreid/apache-document-root/main/chipper/js/grunt/Gruntfile.js
  278:11  error  Possible race condition: `minifyOptions.minify` might be assigned based on an outdated state of `minifyOptions`          require-atomic-updates
  279:11  error  Possible race condition: `minifyOptions.babelTranspile` might be assigned based on an outdated state of `minifyOptions`  require-atomic-updates
  280:11  error  Possible race condition: `minifyOptions.uglify` might be assigned based on an outdated state of `minifyOptions`          require-atomic-updates
  281:11  error  Possible race condition: `minifyOptions.isDebug` might be assigned based on an outdated state of `minifyOptions`         require-atomic-updates

/Users/samreid/apache-document-root/main/phet-io-wrappers/common/js/CredentialsDialog.tsx
  201:5  error  Possible race condition: `fetchOptions.headers` might be assigned based on an outdated state of `fetchOptions`      require-atomic-updates
  208:7  error  Possible race condition: `storedCredentials` might be reassigned based on an outdated value of `storedCredentials`  require-atomic-updates

/Users/samreid/apache-document-root/main/scenery/doc/scenery-sandbox.js
  127:9  error  Possible race condition: `scene.children` might be assigned based on an outdated state of `scene`  require-atomic-updates
  129:9  error  Possible race condition: `scene.opacity` might be assigned based on an outdated state of `scene`   require-atomic-updates

/Users/samreid/apache-document-root/main/tangible/js/mediaPipe/MediaPipe.ts
  260:13  error  Possible race condition: `handsSending` might be reassigned based on an outdated value of `handsSending`    require-atomic-updates
  265:13  error  Possible race condition: `failedOnFrame` might be reassigned based on an outdated value of `failedOnFrame`  require-atomic-updates

/Users/samreid/apache-document-root/main/utterance-queue/js/UtteranceQueueTests.ts
  302:5  error  Possible race condition: `firstUtterance.priorityProperty.value` might be assigned based on an outdated state of `firstUtterance`  require-atomic-updates
  394:5  error  Possible race condition: `thirdUtterance.priorityProperty.value` might be assigned based on an outdated state of `thirdUtterance`  require-atomic-updates
  414:5  error  Possible race condition: `firstUtterance.priorityProperty.value` might be assigned based on an outdated state of `firstUtterance`  require-atomic-updates
  524:5  error  Possible race condition: `firstUtterance.priorityProperty.value` might be assigned based on an outdated state of `firstUtterance`  require-atomic-updates

✖ 15 problems (15 errors, 0 warnings)

Fatal error: Lint failed
Fatal error: perennial grunt "--repo=circuit-construction-kit-common" "lint-everything" "--disable-eslint-cache" failed with code 1

[liammulh]

We currently have 15 failures for this lint rule, so it cannot be enabled without code changes

Understood. I am not sure why this issue is being tracked in the Rosetta repository since it no longer applies to Rosetta. I am going to transfer it to Chipper.

[samreid]

I enabled the rule in the root eslint.config.mjs, added disable directives at usage sites, and will assign file authors that may wish to address these.