Repository: i18next/i18next-parser (js)

Issue:
Regression with version 8.0.11
## 🐛 Bug Report

After upgrade to 8.0.11, this command fail:
```
i18next 'src/**/*.{ts,tsx}' --config i18next-parser.config.js --fail-on-update --silent
```

With

```
> i18next 'src/**/*.{ts,tsx}' --config i18next-parser.config.js --fail-on-update --silent

  [error]   Some keys were sorted and failOnUpdate option is enabled. Exiting...
npm ERR! Lifecycle script `i18n:check` failed with error: 
npm ERR! Error: command failed
```

Sort is enabled.

If I remove `--fail-on-update`, no file are updated, so no need to sort anything.

## To Reproduce

Run the command above with this config file:

```javascript

module.exports = {
  contextSeparator: '_',
  createOldCatalogs: false,
  defaultNamespace: 'translation',
  defaultValue: '',
  indentation: 2,
  keepRemoved: false,
  keySeparator: '.',
  lexers: {
    hbs: ['HandlebarsLexer'],
    handlebars: ['HandlebarsLexer'],

    htm: ['HTMLLexer'],
    html: ['HTMLLexer'],

    mjs: ['JavascriptLexer'],
    ts: ['JsxLexer'],
    jsx: ['JsxLexer'],
    tsx: ['JsxLexer'],

    default: ['JavascriptLexer'],
  },
  lineEnding: 'lf',
  locales: ['en', 'fr'],
  namespaceSeparator: ':',
  output: 'src/locales/$LOCALE/$NAMESPACE.json',
  pluralSeparator: '_',
  input: undefined,
  sort: true,
  skipDefaultValues: false,
  useKeysAsDefaultValue: false,
  verbose: false,
  failOnWarnings: false,
  failOnUpdate: false,
  customValueTemplate: null,
  resetDefaultValueLocale: null,
  i18nextOptions: null,
};
```

## Expected behavior

No file need to be sorted so I should not have any error.

## Your Environment

- *runtime version*: node v21.5.0
- *i18next version*: 23.7.12
- *i18next-parser version*: 8.11.0
- *os*: Mac


Pull request description:
Fix --fail-on-update false negatives from new sort code.
This replaces the code that previously tried to check for sorting via a wrapped `compare` function with a much simpler approach: JSON encode the old and (sorted) new version and see if they match. This code only runs if we have not already seen an update to the keys/contents before, so the performance implications are minimal.

This also adds a test to ensure that false negatives don't occur (i.e. it checks the --fail-on-update behavior against a file that is not being updated and was already sorted).

Fixes #955.

### Why am I submitting this PR

The previous update broke --fail-on-update, making it always fail if sorting was configured.

### Does it fix an existing ticket?

Yes #955

### Checklist

- [x] only relevant code is changed (make a diff before you submit the PR)
- [x] tests are included and pass: `yarn test` (see [details here](https://github.com/i18next/i18next-parser/blob/master/docs/development.md#tests))
- [x] documentation is changed or added


Patch:
diff --git a/src/transform.js b/src/transform.js
index 4a2ca2b..3cd7f7d 100644
--- a/src/transform.js
+++ b/src/transform.js
@@ -298,21 +298,21 @@ export default class i18nTransform extends Transform {
             typeof sort === 'function'
               ? sort
               : makeDefaultSort(this.options.pluralSeparator)
-          let wrappedCompare = this.options.failOnUpdate
-            ? (...args) => {
-                const result = compare(...args)
-                if (result > 0) {
-                  this.parserHadSortUpdate = true
-                  wrappedCompare = compare
-                }
-                return result
-              }
-            : compare
           maybeSortedNewCatalog = sortKeys(newCatalog, {
             deep: true,
-            compare: wrappedCompare,
+            compare,
           })
+
           maybeSortedOldCatalog = sortKeys(oldCatalog, { deep: true, compare })
+
+          if (this.options.failOnUpdate && !this.parserHadUpdate) {
+            // No updates to the catalog, so we do the deeper check to ensure it is also sorted.
+            // This is easiest to accomplish by simply converting both objects to JSON, as we'd have
+            // to do a deep comparison anyway
+            this.parserHadSortUpdate =
+              JSON.stringify(maybeSortedNewCatalog) !==
+              JSON.stringify(existingCatalog)
+          }
         }
 
         // push files back to the stream


Test patch:
diff --git a/test/locales/en/test_sort.json b/test/locales/en/test_sort.json
deleted file mode 100644
index 95b4334..0000000
--- a/test/locales/en/test_sort.json
+++ /dev/null
@@ -1,4 +0,0 @@
-{
-  "second": "second",
-  "first": "first"
-}
diff --git a/test/locales/en/test_sort_sorted.json b/test/locales/en/test_sort_sorted.json
new file mode 100644
index 0000000..3eeadf5
--- /dev/null
+++ b/test/locales/en/test_sort_sorted.json
@@ -0,0 +1,8 @@
+{
+  "first": "first",
+  "second": "second",
+  "third": {
+    "a": "a",
+    "b": "b"
+  }
+}
diff --git a/test/locales/en/test_sort_unsorted.json b/test/locales/en/test_sort_unsorted.json
new file mode 100644
index 0000000..d5c1f74
--- /dev/null
+++ b/test/locales/en/test_sort_unsorted.json
@@ -0,0 +1,8 @@
+{
+  "second": "second",
+  "first": "first",
+  "third": {
+    "b": "b",
+    "a": "a"
+  }
+}
diff --git a/test/parser.test.js b/test/parser.test.js
index 3da76c7..0a74c76 100644
--- a/test/parser.test.js
+++ b/test/parser.test.js
@@ -627,7 +627,10 @@ describe('parser', () => {
       sort: true,
     })
     const fakeFile = new Vinyl({
-      contents: Buffer.from("t('test_sort:first'), t('test_sort:second')"),
+      contents: Buffer.from(
+        "t('test_sort_unsorted:first'), t('test_sort_unsorted:second'), " +
+          "t('test_sort_unsorted:third.a'), t('test_sort_unsorted:third.b')"
+      ),
       path: 'file.js',
     })
 
@@ -649,6 +652,35 @@ describe('parser', () => {
     i18nextParser.end(fakeFile)
   })
 
+  it('does not fail on sort with already sorted keys with failOnUpdate option', (done) => {
+    const i18nextParser = new i18nTransform({
+      output: 'test/locales/$LOCALE/$NAMESPACE.json',
+      failOnUpdate: true,
+      locales: ['en'],
+      sort: true,
+    })
+    const fakeFile = new Vinyl({
+      contents: Buffer.from(
+        "t('test_sort_sorted:first'), t('test_sort_sorted:second'), " +
+          "t('test_sort_sorted:third.a'), t('test_sort_sorted:third.b')"
+      ),
+      path: 'file.js',
+    })
+
+    const realExit = process.exit
+    process.exit = (exitCode) => {
+      process.exit = realExit
+      assert.fail('Should not have called process.exit with ' + exitCode)
+    }
+
+    i18nextParser.once('end', () => {
+      done()
+    })
+
+    i18nextParser.end(fakeFile)
+    i18nextParser.resume()
+  })
+
   it('restores translations from the old catalog', (done) => {
     const i18nextParser = new i18nTransform({
       output: 'test/locales/$LOCALE/$NAMESPACE.json',