Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change all TS/JS uses of VerseRef and ScriptureReference to SerializedVerseRef #1355

Open
4 tasks
lyonsil opened this issue Nov 26, 2024 · 0 comments
Open
4 tasks

Comments

@lyonsil
Copy link
Member

lyonsil commented Nov 26, 2024

We are currently using VerseRef and ScriptureReference in our TS/JS code to represent a location in scripture. However, they both suffer from problems, albeit different ones.

VerseRef:

  • Can serialize to JSON properly using JSON.stringify(), but when deserialized doesn't become a VerseRef again automatically using JSON.parse() since VerseRef is a class.
  • VerseRef objects fail instanceof checks when passed between realms (MDN term - think "security domain").
  • JS class objects in general can have unexpected behavior with this and context loss if not handled carefully.

ScriptureReference:

  • Does not include versification
  • Cannot represent more complex ideas like combined verses that exist in some translations where more than 1 version from the source is turned into a single unit in the destination. In a VerseRef this might look like MAT 1:2-3.

SerializedVerseRef already exists and doesn't suffer from any of these problems. Its main downfall is that it doesn't include its own functions to do things like convert a book ID to a book number, but there are helper classes (e.g., Canon) that can help.

Ira, TJ, and I agreed that using SerializedVerseRef is probably best for passing around pointers to scripture locations. In specific cases where we know a VerseRef will stay within a module we can always convert a SerializedVerseRef into a VerseRef to get access to the helper functions.

To Do:

  • Convert all uses of ScriptureReference to SerializedVerseRef in our TS/JS code.
  • Remove the ScriptureReference type altogether from platform-bible-utils.
  • Review all uses of VerseRef and convert them to SerializedVerseRef if they involve any data flowing in or out of the module. For internal uses within a module, uses of VerseRef can remain as-is.
  • Revert changes to the check aggregator and deserialize from Fix VerseRefs not working as expected with check ranges #1353.

Note that C# code does not need to change. The C# serialization routines already handle VerseRefs appropriately, and C# VerseRef objects don't have the same problems as JS VerseRef objects. The two runtimes handle object types fundamentally differently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 📥 For Consideration
Development

No branches or pull requests

1 participant