Implemented ScriptureForgeAPI and created Slingshot PDPF and PDP

[tjcouch-sil]

Requires https://github.com/paranext/paranext-core/tree/2192-support-slingshot to run

---

This change is 

[lyonsil]

Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @tjcouch-sil)

---

a discussion (no related file):
Looks good overall - just one small question

---

src/scripture-forge/package.json line 40 at r1 (raw file):

  },
  "devDependencies": {
    "@biblionexus-foundation/scripture-utilities": "~0.0.7",

Why ~ and not ^?

[tjcouch-sil]

Reviewable status: all files reviewed, 1 unresolved discussion

---

src/scripture-forge/package.json line 40 at r1 (raw file):

Previously, lyonsil (Matt Lyons) wrote…

Why ~ and not ^?

Ira decided to do this everywhere to indicate more clearly that pre-1.0 versions assume breaking changes. I just copied what he did.

[irahopkinson]

Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions

---

src/scripture-forge/src/projects/slingshot-project-data-provider-engine-factory.model.ts line 9 at r1 (raw file):

  SLINGSHOT_PROJECT_INTERFACES,
} from './slingshot-project-data-provider-engine.model';
import ScriptureForgeAPI, { ScriptureForgeProjectInfo } from './scripture-forge-api.model';

NIT if we are following our naming convention ScriptureForgeAPI should be ScriptureForgeApi'. Here and everywhere else you use API`.

---

src/scripture-forge/src/projects/slingshot-project-data-provider-engine-factory.model.ts line 33 at r1 (raw file):

  async getAvailableProjects(): Promise<ProjectMetadataWithoutFactoryInfo[]> {
    const projectsInfo = await this.scriptureForgeAPI.getProjects();
    return projectsInfo

BTW I would use an early return here rather than the ternary for readability.

---

src/scripture-forge/src/projects/slingshot-project-data-provider-engine.model.ts line 23 at r1 (raw file):

 */
// TypeScript is upset without `satisfies` here because `as const` makes the array readonly but it

NIT disjointed comment - remove blank line

---

src/scripture-forge/src/projects/slingshot-project-data-provider-engine.model.ts line 177 at r1 (raw file):

      case 'platform.fullName':
        return (
          // TypeScript doesn't realize ProjectSettingName is 'platform.name' in this case for some reason

BTW there is a mismatch between this comment and the case value (here and the next case). Is that intentional?

Code quote:

'platform.name'

---

src/scripture-forge/src/projects/scripture-forge-api.model.ts line 31 at r1 (raw file):

  name: string;
  shortName: string;
  languageRegion: string | null;

BTW is there some reason we are using null instead of undefined?

---

src/scripture-forge/src/projects/scripture-forge-api.model.ts line 67 at r1 (raw file):

    // We can infer that this json will come back as a ScriptureForgeProjectInfo[]
    // eslint-disable-next-line no-type-assertion/no-type-assertion
    if (projectResponse.ok) return (await projectResponse.json()) as ScriptureForgeProjectInfo[];

NIT IMHO readability would improve by using an early return, ie:

    if (!projectResponse?.ok) return undefined;

    // We can infer that this json will come back as a ScriptureForgeProjectInfo[]
    // eslint-disable-next-line no-type-assertion/no-type-assertion
    return (await projectResponse.json()) as ScriptureForgeProjectInfo[];

[irahopkinson]

Reviewable status: all files reviewed, 7 unresolved discussions

---

src/scripture-forge/package.json line 40 at r1 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

Ira decided to do this everywhere to indicate more clearly that pre-1.0 versions assume breaking changes. I just copied what he did.

This fixed an issue we had when we updated the utilities version but the current version of the editor had a previous version since pre-release versions behave slightly differently in SemVer.

[tjcouch-sil]

Reviewable status: all files reviewed, 6 unresolved discussions

---

src/scripture-forge/src/projects/scripture-forge-api.model.ts line 31 at r1 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

BTW is there some reason we are using null instead of undefined?

The API gives null; we don't control this. I got this type from Nathaniel's API code sample.

This type is retrieved in this API class and interpreted in the PDP class, but it isn't exposed anywhere. The nulls stay in here.

---

src/scripture-forge/src/projects/scripture-forge-api.model.ts line 67 at r1 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

NIT IMHO readability would improve by using an early return, ie:

    if (!projectResponse?.ok) return undefined;

    // We can infer that this json will come back as a ScriptureForgeProjectInfo[]
    // eslint-disable-next-line no-type-assertion/no-type-assertion
    return (await projectResponse.json()) as ScriptureForgeProjectInfo[];

Yep, sounds good. Thanks! (will adjust for later PR)

---

src/scripture-forge/src/projects/slingshot-project-data-provider-engine-factory.model.ts line 9 at r1 (raw file):
I believe we are inconsistent about initialism/acronym capitalization right now. For example, in platform-scripture.d.ts, we have USJ everywhere. But in the editor and utilities packages, it's Usj. Might be good to discuss and decide on a rule as all I see in the code style guide is as follows:

• Avoid abbreviations or contractions as part of identifier names. Don't use any acronyms that are not widely accepted, and even if they are, only when necessary.

Seems there is no particular consensus. JavaScript can't even make up its mind within the same identifier: XMLHttpRequest 😂

---

src/scripture-forge/src/projects/slingshot-project-data-provider-engine-factory.model.ts line 33 at r1 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

BTW I would use an early return here rather than the ternary for readability.

Yep, sounds great

---

src/scripture-forge/src/projects/slingshot-project-data-provider-engine.model.ts line 23 at r1 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

NIT disjointed comment - remove blank line

Oops! How did that get there? 😂 thanks

---

src/scripture-forge/src/projects/slingshot-project-data-provider-engine.model.ts line 177 at r1 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

BTW there is a mismatch between this comment and the case value (here and the next case). Is that intentional?

Nope! Good catch!

[tjcouch-sil]

Reviewable status: all files reviewed, 5 unresolved discussions

---

src/scripture-forge/src/projects/slingshot-project-data-provider-engine-factory.model.ts line 9 at r1 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

I believe we are inconsistent about initialism/acronym capitalization right now. For example, in platform-scripture.d.ts, we have USJ everywhere. But in the editor and utilities packages, it's Usj. Might be good to discuss and decide on a rule as all I see in the code style guide is as follows:

• Avoid abbreviations or contractions as part of identifier names. Don't use any acronyms that are not widely accepted, and even if they are, only when necessary.

Seems there is no particular consensus. JavaScript can't even make up its mind within the same identifier: XMLHttpRequest 😂

We decided on following .NET's conventions https://github.com/paranext/paranext/wiki/Code-Style-Guide/_compare/c58ab02e0db25e55849112c0b56cbec7042441c0...573ab73ba0f8724289ce3e78b76ea40fcf5f86e4 will adjust in my next PR :)