feat: Added React Demo webApp and deployment workflow#73
feat: Added React Demo webApp and deployment workflow#73rockyRaccoon13 wants to merge 159 commits intoni:mainfrom
Conversation
|
@rockyRaccoon13 let's review this PR and #72 together at our next meeting. To other repo owners, no need to review these PRs yet, I'll help the team iterate on them first. |
jattasNI
left a comment
There was a problem hiding this comment.
Here's my first batch of feedback. Overall it's going in a good direction: I was able to build the app locally and make the API call, which is a success for your implementation and your docs.
This feedback is mostly about docs and infrastructure high level organization and app capabilities. Once we get that dialed in I'll get finer grained on code.
examples/Web Applications/Framework Examples/React/ApiKeyAuthApp/eslint.config.js
Outdated
Show resolved
Hide resolved
examples/web_apps/Framework_Examples/React/ApiKeyAuthApp/src/.gitignore
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,208 @@ | |||
| /* @use '@ni/nimble-components/dist/fonts' as *; | |||
There was a problem hiding this comment.
Let's remove commented out code to reduce clutter.
Could you also take a pass at consistent spacing (avoid lots of repeated newlines, use consistent spaces after CSS selectors)?
| background-color: #f5f5f5; | ||
| overflow-y: auto; | ||
| margin: 0; | ||
| font-family: var(--nimble-font-family-body); |
There was a problem hiding this comment.
It doesn't look like this is working. If I inspect in dev tools it says these variables are not set.
I suspect the issue is that you need to import tokens like the Nimble react example app does. It also imports fonts in the app wide index.scss file, not sure if that's needed too.
| @@ -0,0 +1,208 @@ | |||
| /* @use '@ni/nimble-components/dist/fonts' as *; | |||
There was a problem hiding this comment.
Does React have a configuration that lets you use SCSS instead of CSS? We tend to prefer SCSS because it gives better compile-time checking of variable names.
If there's not an obvious / cheap way to do this, I'm ok to defer it to a follow up task.
There was a problem hiding this comment.
Vite definitely does out of the box, the nimble react-client-app would be a good example:
https://github.com/ni/nimble/tree/main/packages/react-workspace/react-client-app
https://github.com/ni/nimble/blob/main/packages/react-workspace/react-client-app/src/index.scss
There was a problem hiding this comment.
nimble went with @vitejs/plugin-react-swc which skips babel and is super fast with a tradeoff of a bunch of compile checks instead of @vitejs/plugin-react but either way SCSS should be supported
Co-authored-by: Jesse Attas <jattasNI@users.noreply.github.com>
Co-authored-by: Jesse Attas <jattasNI@users.noreply.github.com>
Justification
Added a React demo web application using Vite to demonstrate how a customer might create a webApp and deploy it to SystemLink. The webApp demos how to call SystemLink APIs both in development (locally) and in production (within a SystemLink website).
The github actions webApp workflow automates building, linting, and deployment to the SystemLink website. The workflow also outputs a .nipkg artifact of the webApp to github. See
/.github/workflows/webapp_deploy.ymlImplementation
Webapp Design
examples/web_apps/Framework_Examples/React/ApiKeyAuthAppRunning in Development (locally) vs Production (hosted in SystemLink website)
.env.developmentand.env.production) files allow variable domain for fetches (dev will fetch from the local proxy server, prod will fetch from the hosting website)ApiServiceProxy -- Required to run web app locally
token authentication when the web app is hosted in the SystemLink website
ApiServiceProxy/README.mdProduction -- Running inside SystemLink Website
ApiKeyAuthApp/README.md./deployDev.shWorkflow -- Building, Linting, Deployment to SystemLink website
package.json(npm ci, npm lint, npm build).SystemLink CLI (SL CLI) integration in Workflow
SL_API_URLSL_API_KEYSL_WEBSITE_URLSL_WORKSPACETesting
Local env
Within SystemLink website
Workflow
Checklist