Enable webview styles

[irahopkinson]

• Build components with vite
  • scripture utils moved to papi-components
• Install "papi-components" package
• Limit C# message length
• Remove papi.react.components

---

This change is 

[tjcouch-sil]

Thanks for the hard work on this!! Exciting :) Did anything you did fix the webview bundle size?

Reviewed 51 of 58 files at r1, 7 of 7 files at r2, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @irahopkinson)

---

package.json line 58 at r2 (raw file):

    "csharp:tool:restore": "cd c-sharp && dotnet tool restore",
    "postinstall": "ts-node .erb/scripts/check-native-dep.js && electron-builder install-app-deps && cd extensions && npm install && cd .. && cross-env NODE_ENV=development TS_NODE_TRANSPILE_ONLY=true webpack --config ./.erb/configs/webpack.config.renderer.dev.dll.ts",
    "lint": "npm run build:types && npm run build:extensions && cross-env NODE_ENV=development eslint . --ext .js,.jsx,.ts,.tsx && cd lib/papi-dts && npm run lint && cd ../../extensions && npm run lint",

Do we need to lint papi-components here?

---

package.json line 60 at r2 (raw file):

    "lint": "npm run build:types && npm run build:extensions && cross-env NODE_ENV=development eslint . --ext .js,.jsx,.ts,.tsx && cd lib/papi-dts && npm run lint && cd ../../extensions && npm run lint",
    "lint:config": "cross-env NODE_ENV=development eslint --print-config .eslintrc.js > .eslintConfig.json",
    "lint:staged": "npm run build:types && npm run build:extensions && lint-staged -q && cd lib/papi-dts && npm run lint:staged && cd ../../extensions && npm run lint:staged",

Do we need to lint papi-components here?

---

lib/papi-components/.eslintrc line 5 at r2 (raw file):

  "parser": "@typescript-eslint/parser",
  "plugins": ["@typescript-eslint", "prettier"],
  "extends": [

Is there a reason we're not extending the main eslintrc here? You do have to do some special stuff to override some stuff from the main one. You can see the way it can be extended in extensions/.eslintrc.cjs if that's the only problem.

---

lib/papi-components/.gitignore line 6 at r2 (raw file):

# production
build
# dist # We want these commited so developers don't need to build the package.

I think this warrants a discussion among the core team. We are building papi-dts and extensions on the server, and I expect we would want to do the same here.

---

lib/papi-components/package.json line 4 at r2 (raw file):

  "name": "papi-components",
  "version": "0.0.1",
  "description": "Components available on Paranext's PAPI - package to include styles sheets.",

Probably should change this description to something like "React components to use in Paranext with matching styles" or something.

---

src/stories/button.stories.tsx line 2 at r2 (raw file):

import type { Meta, StoryObj } from '@storybook/react';
import { Button } from 'papi-components';

Think we should put this storybook stuff inside papi-components? Would you mind making an issue for this?

---

src/stories/table.stories.tsx line 9 at r2 (raw file):

  TableCopyEvent,
  TablePasteEvent,
} from 'papi-components/src/table.component';

This is a bit of a strange import... Can we import this from papi-components?

[irahopkinson]

No but I did discover that it is controlled by extensions\vite\vite-web-view.config.ts. Even when I removed generation of source maps in the papi-components, they are brought back by extensions\vite\vite-web-view.config.ts. The source maps could be so large because we may not be excluding the actual sources from the maps but I didn't investigate that. But a map could be removed there individually if needed (see issue comment) but I think we will need to do a prodcution configuration that removes all source maps (but not until next year).

Reviewable status: 57 of 58 files reviewed, 1 unresolved discussion (waiting on @tjcouch-sil)

---

package.json line 58 at r2 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

Do we need to lint papi-components here?

No, since papi-components is its own npm package, we lint and build it separately. See my larger comment below.

---

package.json line 60 at r2 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

Do we need to lint papi-components here?

No, since papi-components is its own npm package, we lint and build it separately. See my larger comment below.

---

lib/papi-components/.eslintrc line 5 at r2 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

Is there a reason we're not extending the main eslintrc here? You do have to do some special stuff to override some stuff from the main one. You can see the way it can be extended in extensions/.eslintrc.cjs if that's the only problem.

No, since papi-components is its own npm package, we lint and build it separately. See my comment below.

---

lib/papi-components/.gitignore line 6 at r2 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

I think this warrants a discussion among the core team. We are building papi-dts and extensions on the server, and I expect we would want to do the same here.

I think that papi-components is completely different than papi-dts. papi-dts should never be its own npm package and can change on a minute by minute basis when coding. It therefore needs to be built with everything else. For that reason you could argue that the generated file could/should be ignored from git (but its only one file so fine as is). Because papi-components is an npm package (even though it's not published externally yet), changes to it can be made and "published" (currently that means commited but could be ignored from git once we publish to npm) on their own cycle. Happy to have a core team discussion on this.

---

lib/papi-components/package.json line 4 at r2 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

Probably should change this description to something like "React components to use in Paranext with matching styles" or something.

Done.

---

src/stories/button.stories.tsx line 2 at r2 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

Think we should put this storybook stuff inside papi-components? Would you mind making an issue for this?

I thought about that but I think we will want to storybook things outside of papi-components such as dock window items and menu components, therefore we want to keep storybook at the app root level. Hopefully we can group things in storybook to organize them as we grow.

---

src/stories/table.stories.tsx line 9 at r2 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

This is a bit of a strange import... Can we import this from papi-components?

Yes, this is intentionally left for the extensions team to fix since table wasn't previously included properly. I added the checkbox in this change as another example for them but left the table component as an excercise for them to learn about including components. We need to leave them something to do for issue #158.

[tjcouch-sil]

Reviewed 1 of 1 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @irahopkinson)

---

lib/papi-components/.eslintrc line 5 at r2 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

No, since papi-components is its own npm package, we lint and build it separately. See my comment below.

Does that mean we shouldn't use the same (or similar) linting rules in papi-components as for paranext-core? I'm not sure I understand the thought process here. My thought is we might as well reduce duplication and keep configuration tight. Could you explain your thoughts more please? Thanks!

---

lib/papi-components/.gitignore line 6 at r2 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

I think that papi-components is completely different than papi-dts. papi-dts should never be its own npm package and can change on a minute by minute basis when coding. It therefore needs to be built with everything else. For that reason you could argue that the generated file could/should be ignored from git (but its only one file so fine as is). Because papi-components is an npm package (even though it's not published externally yet), changes to it can be made and "published" (currently that means commited but could be ignored from git once we publish to npm) on their own cycle. Happy to have a core team discussion on this.

Actually, I think we do want papi-dts to be its own published npm package as well. That way, people can develop extensions without needing paranext-core installed and built and such. It would be worth having a meeting to figure out how we want these dependencies to look. I will try to set one up soon.

---

src/stories/button.stories.tsx line 2 at r2 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

I thought about that but I think we will want to storybook things outside of papi-components such as dock window items and menu components, therefore we want to keep storybook at the app root level. Hopefully we can group things in storybook to organize them as we grow.

I was thinking two storybooks, one for components and one for normal core stuff. It's not a big deal right now either way, though.

---

src/stories/table.stories.tsx line 9 at r2 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

Yes, this is intentionally left for the extensions team to fix since table wasn't previously included properly. I added the checkbox in this change as another example for them but left the table component as an excercise for them to learn about including components. We need to leave them something to do for issue #158.

I'm not sure I understand, but I trust your judgment :) would you mind please describing this issue in #158 if you think the changes should be there?

[irahopkinson]

Reviewable status: 43 of 59 files reviewed, 1 unresolved discussion (waiting on @tjcouch-sil)

---

lib/papi-components/.eslintrc line 5 at r2 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

Does that mean we shouldn't use the same (or similar) linting rules in papi-components as for paranext-core? I'm not sure I understand the thought process here. My thought is we might as well reduce duplication and keep configuration tight. Could you explain your thoughts more please? Thanks!

I was thinking since this might be removed from Core eventually then it needed to define its own rules (even if they are a duplicate of core's rules) but since we aren't doing that yet (or even ever), I've extended it off core as you suggest.

---

lib/papi-components/.gitignore line 6 at r2 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

Actually, I think we do want papi-dts to be its own published npm package as well. That way, people can develop extensions without needing paranext-core installed and built and such. It would be worth having a meeting to figure out how we want these dependencies to look. I will try to set one up soon.

I think we resolved this at the core meeting. I'm also doing the linting at build time as discussed.

---

src/stories/table.stories.tsx line 9 at r2 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

I'm not sure I understand, but I trust your judgment :) would you mind please describing this issue in #158 if you think the changes should be there?

Done. See #158 (comment)

[tjcouch-sil]

Awesome. Thanks for the hard work on this! Just the one thing about eslintrc to follow up, but it's not blocking as I assume you ran the scripts and such after extending. Let's go ahead and get this in. If there are any eslintrc problems, maybe a follow-up PR can solve the problems or something.

Reviewed 16 of 16 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @irahopkinson)

---

lib/papi-components/.eslintrc line 5 at r2 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

I was thinking since this might be removed from Core eventually then it needed to define its own rules (even if they are a duplicate of core's rules) but since we aren't doing that yet (or even ever), I've extended it off core as you suggest.

Cool :) I am surprised to see you didn't have to do the webpack disable thing I did in the extensions eslintrc. Do you get any weird errors or warnings in the console related to webpack when linting or building papi-components now that you're extending the main eslintrc?

---

lib/papi-components/.gitignore line 6 at r2 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

I think we resolved this at the core meeting. I'm also doing the linting at build time as discussed.

Yes, here are my notes from the meeting. Decisions in bold:

npm packages inside paranext-core

• build/lint with paranext-core?
  • Tim - everything changing so fast that it would be annoying to publish separately. Once things settle, use published packages. Can we keep up with quick changes on a published package?
  • Alternatively do separate build for papi-dts and publish and then build paranext from that
  • Since paranext-core depends on papi-components (not other way around as is the case with papi-dts), when changes happen to papi-components, they will have to be built to see any changes in paranext-core. So let's make linting happen on building papi-components and leave it at that (meaning updated build files will be assumed to mean the source has been linted)
• built code checked in?
  • Keep it in so we don't have to build every pre-commit and such
    • Makes it easier for us for now. We can probably remove later when we publish
  • Go ahead and publish since we have people working on it
    • We're not at external extension ready. Those who are working are incidentally doing so

Thanks for following up on these decisions!

---

lib/papi-components/package.json line 34 at r5 (raw file):

    "build:basic": "tsc && vite build && dts-bundle-generator --config ./dts-bundle-generator.config.ts",
    "build": "npm run build:basic && npm run format:scripts && npm run lint:scripts && npm run format:styles && npm run lint:styles",
    "watch": "tsc && vite build --watch",

I think we may need to be watchful for when devs run watch but don't ever build. But I don't have a better solution right now. This will hopefully work just fine for now.

---

src/stories/table.stories.tsx line 9 at r2 (raw file):

Previously, irahopkinson (Ira Hopkinson) wrote…

Done. See #158 (comment)

Thank you!! :)

[irahopkinson]

lib/papi-components/.eslintrc line 5 at r2 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

Cool :) I am surprised to see you didn't have to do the webpack disable thing I did in the extensions eslintrc. Do you get any weird errors or warnings in the console related to webpack when linting or building papi-components now that you're extending the main eslintrc?

Yeah, I first copied everything over. I think what made a difference for me is I added "root": true,, after that I removed every setting one at a time to see what was needed and only alwaysTryTypes remains.