]> Lady’s Gitweb - Etiquette/commitdiff
Always return (the same) objects for langstrings
authorLady <redacted>
Wed, 14 Jun 2023 02:02:55 +0000 (19:02 -0700)
committerLady <redacted>
Wed, 14 Jun 2023 02:08:05 +0000 (19:08 -0700)
Instead of returning a string literal in some cases and a `String`
object in others, always return an object when processing language
strings. Use a cache and registry to always return the same object for
any given value and language, to allow `Set` operations to work as
expected.

model.js
model.test.js

index 48e68134bb1bdcf9ff9531169c6c70a901c0065d..ba71361890494f3e3c7aaa4369af83d50b8e8440 100644 (file)
--- a/model.js
+++ b/model.js
@@ -1374,9 +1374,11 @@ class Tag {
 
 const {
   /**
-   * Returns the provided value converted into either a plain string
-   * primitive or an object with `.["@value"]` and `.["@language"]`
-   * properties.
+   * Returns the provided value converted into a `String` object with
+   * `.["@value"]` and `.["@language"]` properties.
+   *
+   * The same object will be returned for every call with an equivalent
+   * value.
    *
    * TODO: Ideally this would be extracted more fully into an R·D·F
    * library.
@@ -1385,23 +1387,85 @@ const {
    */
   langString,
 } = (() => {
+  /**
+   * Returns the language string object corresponding to the provided
+   * value and language.
+   */
+  const getLangString = (value, language = "") => {
+    const valueMap = languageMap[language] ??= Object.create(null);
+    const literal = valueMap[value]?.deref();
+    if (literal != null) {
+      // There is already an object corresponding to the provided value
+      // and language.
+      return literal;
+    } else {
+      // No object already exists corresponding to the provided value
+      // and language; create one.
+      const result = Object.preventExtensions(
+        Object.create(String.prototype, {
+          "@value": {
+            enumerable: true,
+            value,
+          },
+          "@language": {
+            enumerable: !!language,
+            value: language || null,
+          },
+          language: { enumerable: false, get: getLanguage },
+          toString: { enumerable: false, value: toString },
+          valueOf: { enumerable: false, value: valueOf },
+        }),
+      );
+      const ref = new WeakRef(result);
+      langStringRegistry.register(result, { ref, language, value });
+      valueMap[value] = ref;
+      return result;
+    }
+  };
+
   /** Returns the `.["@language"]` of this object. */
   const getLanguage = Object.defineProperty(
     function () {
-      return this["@language"];
+      return this["@language"] || null;
     },
     "name",
     { value: "get language" },
   );
 
+  /**
+   * A `FinalizationRegistry` for language string objects.
+   *
+   * This simply cleans up the corresponding `WeakRef` in the language
+   * map.
+   */
+  const langStringRegistry = new FinalizationRegistry(
+    ({ ref, language, value }) => {
+      const valueMap = languageMap[language];
+      if (valueMap?.[value] === ref) {
+        delete valueMap[value];
+      } else {
+        /* do nothing */
+      }
+    },
+  );
+
+  /**
+   * An object whose own values are an object mapping values to
+   * language string objects for the language specified by the key.
+   */
+  const languageMap = Object.create(null);
+
   /** Returns the `.["@value"]` of this object. */
   const toString = function () {
     return this["@value"];
   };
 
-  /** Returns the `.["@value"]` of this object. */
+  /**
+   * Returns this object if it has a `.["@language"]`; otherwise, its
+   * `.["@value"]`.
+   */
   const valueOf = function () {
-    return this["@value"];
+    return this["@language"] ? this : this["@value"];
   };
 
   return {
@@ -1409,37 +1473,15 @@ const {
       Object($) === $
         ? "@value" in $
           ? "@language" in $
-            ? Object.preventExtensions(
-              Object.create(String.prototype, {
-                "@value": {
-                  enumerable: true,
-                  value: `${$["@value"]}`,
-                },
-                "@language": {
-                  enumerable: true,
-                  value: `${$["@language"]}`,
-                },
-                language: { enumerable: false, get: getLanguage },
-                toString: { enumerable: false, value: toString },
-                valueOf: { enumerable: false, value: valueOf },
-              }),
+            ? getLangString(
+              `${$["@value"]}`,
+              `${$["@language"] ?? ""}`,
             )
-            : `${$["@value"]}`
+            : getLangString(`${$["@value"]}`)
           : "language" in $
-          ? Object.preventExtensions(
-            Object.create(String.prototype, {
-              "@value": { enumerable: true, value: `${$}` },
-              "@language": {
-                enumerable: true,
-                value: `${$.language}`,
-              },
-              language: { enumerable: false, get: getLanguage },
-              toString: { enumerable: false, value: toString },
-              valueOf: { enumerable: false, value: valueOf },
-            }),
-          )
-          : `${$}`
-        : `${$ ?? ""}`,
+          ? getLangString(`${$}`, `${$.language ?? ""}`)
+          : getLangString(`${$}`)
+        : getLangString(`${$ ?? ""}`),
   };
 })();
 
index 15a2f54cfa955831457a64d8dcd215e486937ed4..3e9982a7a4fb513495204b5d80608e4d04f2c6a9 100644 (file)
@@ -105,13 +105,13 @@ describe("TagSystem", () => {
     });
 
     it("[[Construct]] defaults the preferred label to the empty string", () => {
-      assertStrictEquals(new Tag().prefLabel, "");
+      assertEquals({ ...new Tag().prefLabel }, { "@value": "" });
     });
 
     it("[[Construct]] correctly sets the preferred label to a simple string", () => {
-      assertStrictEquals(
-        new Tag("RelationshipTag", "Shadow, Me").prefLabel,
-        "Shadow, Me",
+      assertEquals(
+        { ...new Tag("RelationshipTag", "Shadow, Me").prefLabel },
+        { "@value": "Shadow, Me" },
       );
     });
 
@@ -187,7 +187,9 @@ describe("TagSystem", () => {
       });
 
       it("[[Call]] throws if passed an invalid I·R·I", () => {
-        assertThrows(() => {Tag.fromIRI(`bad iri`)});
+        assertThrows(() => {
+          Tag.fromIRI(`bad iri`);
+        });
       });
     });
 
@@ -295,13 +297,10 @@ describe("TagSystem", () => {
           { "@value": "three", "@language": "en" },
         );
         assertEquals(
-          Array.from(
-            tag.altLabels(),
-            ($) => typeof $ == "string" ? $ : { ...$ },
-          ),
+          Array.from(tag.altLabels(), ($) => ({ ...$ })),
           [
-            "one",
-            "two",
+            { "@value": "one" },
+            { "@value": "two" },
             { "@value": "three", "@language": "en" },
           ],
         );
@@ -371,13 +370,10 @@ describe("TagSystem", () => {
           { "@value": "three", "@language": "en" },
         );
         assertEquals(
-          Array.from(
-            tag.hiddenLabels(),
-            ($) => typeof $ == "string" ? $ : { ...$ },
-          ),
+          Array.from(tag.hiddenLabels(), ($) => ({ ...$ })),
           [
-            "one",
-            "two",
+            { "@value": "one" },
+            { "@value": "two" },
             { "@value": "three", "@language": "en" },
           ],
         );
@@ -553,7 +549,10 @@ describe("TagSystem", () => {
         const tag = new Tag();
         tag.addAltLabel("etaoin");
         tag.deleteAltLabel();
-        assertEquals([...tag.altLabels()], ["etaoin"]);
+        assertEquals(
+          Array.from(tag.altLabels(), ($) => ({ ...$ })),
+          [{ "@value": "etaoin" }],
+        );
       });
 
       it("[[Call]] deletes only the provided hidden labels", () => {
@@ -570,7 +569,10 @@ describe("TagSystem", () => {
           { "@value": "three", "@language": "en" },
           { "@value": "four", "@language": "en" },
         );
-        assertEquals([...tag.altLabels()], ["four"]);
+        assertEquals(
+          Array.from(tag.altLabels(), ($) => ({ ...$ })),
+          [{ "@value": "four" }],
+        );
       });
 
       it("[[Call]] returns this", () => {
@@ -621,7 +623,10 @@ describe("TagSystem", () => {
         const tag = new Tag();
         tag.addHiddenLabel("etaoin");
         tag.deleteHiddenLabel();
-        assertEquals([...tag.hiddenLabels()], ["etaoin"]);
+        assertEquals(
+          Array.from(tag.hiddenLabels(), ($) => ({ ...$ })),
+          [{ "@value": "etaoin" }],
+        );
       });
 
       it("[[Call]] deletes only the provided alternative labels", () => {
@@ -638,7 +643,10 @@ describe("TagSystem", () => {
           { "@value": "three", "@language": "en" },
           { "@value": "four", "@language": "en" },
         );
-        assertEquals([...tag.hiddenLabels()], ["four"]);
+        assertEquals(
+          Array.from(tag.hiddenLabels(), ($) => ({ ...$ })),
+          [{ "@value": "four" }],
+        );
       });
 
       it("[[Call]] returns this", () => {
@@ -916,9 +924,9 @@ describe("TagSystem", () => {
       it("[[Set]] sets the preferred label", () => {
         const tag = new Tag();
         tag.prefLabel = "one";
-        assertStrictEquals(tag.prefLabel, "one");
+        assertEquals({ ...tag.prefLabel }, { "@value": "one" });
         tag.prefLabel = { "@value": "two" };
-        assertStrictEquals(tag.prefLabel, "two");
+        assertEquals({ ...tag.prefLabel }, { "@value": "two" });
         tag.prefLabel = { "@value": "three", "@language": "en" };
         assertEquals(
           { ...tag.prefLabel },
This page took 0.032266 seconds and 4 git commands to generate.