From: Lady Date: Wed, 14 Jun 2023 02:02:55 +0000 (-0700) Subject: Always return (the same) objects for langstrings X-Git-Url: https://git.ladys.computer/Etiquette/commitdiff_plain/771a1ca8d8302b01060db00de7f588093e8805ec?ds=inline Always return (the same) objects for langstrings 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. --- diff --git a/model.js b/model.js index 48e6813..ba71361 100644 --- 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(`${$ ?? ""}`), }; })(); diff --git a/model.test.js b/model.test.js index 15a2f54..3e9982a 100644 --- a/model.test.js +++ b/model.test.js @@ -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 },