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.

Sign up for GitHub

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

chore(frontend): remix v2 upgrade #2349

Merged
merged 44 commits into from
Jan 26, 2024
Merged

chore(frontend): remix v2 upgrade #2349

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
094e6f0
chore(frontend): update error boundaries to v2
BlairCurrey Jan 18, 2024
28d892b
chore(frontend): update meta to v2
BlairCurrey Jan 18, 2024
ef8760a
chore(frontend): update headers to v2
BlairCurrey Jan 18, 2024
6228d2c
chore(frontend): update form method to v2
BlairCurrey Jan 18, 2024
ca9ad2c
chore(frontend): update to v2 dev server
BlairCurrey Jan 18, 2024
38cfb94
chore(frontend): update server module format to v2
BlairCurrey Jan 18, 2024
6ded4de
feat(frontend): typecheck cmd
BlairCurrey Jan 19, 2024
1506658
fix(frontend): type only import
BlairCurrey Jan 19, 2024
00f4874
feat(frontend): ugprade remix to v2
BlairCurrey Jan 19, 2024
85b386a
feat: add frontend typecheck to ci
BlairCurrey Jan 19, 2024
3b32e7b
chore(MASE): upgrade routes to remix v2
BlairCurrey Jan 19, 2024
d45879b
chore(MASE): upgrade module format to v2
BlairCurrey Jan 19, 2024
91c22ca
chore(MASE): update to v2 dev server
BlairCurrey Jan 19, 2024
95870de
chore(MASE): update headers to v2
BlairCurrey Jan 19, 2024
b7e53ea
chore(MASE): update form method to v2
BlairCurrey Jan 19, 2024
47ee050
chore(MASE): update meta to v2
BlairCurrey Jan 19, 2024
adda951
chore(MASE): update error/catch boundary to v2
BlairCurrey Jan 19, 2024
3beabe9
feat(MASE): upgrade remix to v2
BlairCurrey Jan 19, 2024
3d115f3
fix(MASE): ts errors
BlairCurrey Jan 19, 2024
50d51c6
feat(MASE): add typecheck cmd to ci
BlairCurrey Jan 19, 2024
37f5fa8
fix(frontend, MASE): remix server start
BlairCurrey Jan 22, 2024
1a3bacb
Merge branch 'main' into bc/1464/remix-v2-updates
BlairCurrey Jan 22, 2024
32e29c1
fix(MASE): eslint
BlairCurrey Jan 22, 2024
29c4a24
refactor(MASE): parseQueryString to URLSearchParams
BlairCurrey Jan 22, 2024
9969315
Update .github/workflows/lint_test_build.yml
BlairCurrey Jan 23, 2024
0c660ae
fix(frontend,MASE): update entry.server request handlers
BlairCurrey Jan 23, 2024
8250581
chore(MASE): format
BlairCurrey Jan 23, 2024
41673bb
chore(MASE): rm unused quotes route
BlairCurrey Jan 23, 2024
f7149aa
Merge branch 'main' into bc/1464/remix-v2-updates
BlairCurrey Jan 24, 2024
632ad8c
Revert "Merge branch 'main' into bc/1464/remix-v2-updates"
BlairCurrey Jan 24, 2024
7a6ad8b
chore: cherry pick merge commit after reverting in f7149aa
BlairCurrey Jan 24, 2024
6b72997
Merge branch 'main' into bc/1464/remix-v2-updates
BlairCurrey Jan 24, 2024
fb4e5b7
fix(frontend,MASE): mobile view
BlairCurrey Jan 24, 2024
dab6502
fix(frontend): no flash on mobile
BlairCurrey Jan 24, 2024
2db091c
refactor: rework new cookie secure env var
BlairCurrey Jan 25, 2024
b35dc09
fix: move env var declaration from dockerfile to compose
BlairCurrey Jan 25, 2024
a51f877
Merge branch 'main' into bc/1464/remix-v2-updates
BlairCurrey Jan 25, 2024
58fb5e4
Merge branch 'main' into bc/1464/remix-v2-updates
BlairCurrey Jan 25, 2024
93c8318
feat(backend): upgrade Rafiki to open payments package v6.5 (#2337)
sabineschaller Jan 23, 2024
230d57e
chore(deps): update dependency autoprefixer to ^10.4.17 (#2352)
renovate[bot] Jan 23, 2024
e358d16
chore: update lockfile
BlairCurrey Jan 25, 2024
05f65f6
chore: rm extra space
BlairCurrey Jan 25, 2024
eaaee0e
chore: rm unnecessary eslint ignore
BlairCurrey Jan 25, 2024
455ebb8
Merge branch 'main' into bc/1464/remix-v2-updates
BlairCurrey Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions packages/documentation/src/content/docs/integration/deployment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,9 @@ Now, the Admin UI can be found on localhost:3010.

#### Frontend

| Variable | Helm Value Name | Default | Description |
| ------------------- | -------------------------------------- | ------------------------------- | ------------------------------------- |
| `GRAPHQL_URL` | frontend.serviceUrls.GRAPHQL_URL | `http://localhost:3001/graphql` | URL for the GraphQL Admin API |
| `OPEN_PAYMENTS_URL` | frontend.serviceUrls.OPEN_PAYMENTS_URL | `http://localhost:3003/` | Open Payments API Endpoint |
| `PORT` | frontend.port | `3005` | Port from which to host the Remix app |
| Variable | Helm Value Name | Default | Description |
| -------------------------------- | -------------------------------------- | ------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `GRAPHQL_URL` | frontend.serviceUrls.GRAPHQL_URL | `http://localhost:3001/graphql` | URL for the GraphQL Admin API |
| `OPEN_PAYMENTS_URL` | frontend.serviceUrls.OPEN_PAYMENTS_URL | `http://localhost:3003/` | Open Payments API Endpoint |
| `PORT` | frontend.port | `3005` | Port from which to host the Remix app |
| `ENABLE_INSECURE_MESSAGE_COOKIE` | | _undefined_ | Values of `true`, `t`, `1` will not use a `secure` message cookie which is required for flash messages to work over http. When not set, the secure flag is set according to `NODE_ENV`. |
2 changes: 1 addition & 1 deletion packages/frontend/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,4 @@ COPY --from=builder /home/rafiki/packages/frontend/public ./packages/frontend/pu

WORKDIR /home/rafiki/packages/frontend

CMD ["sh", "-c", "NODE_ENV=production COOKIE_SECURE=false ./node_modules/.bin/remix-serve ./build/index.js"]
CMD ["sh", "-c", "NODE_ENV=production ENABLE_INSECURE_MESSAGE_COOKIE=true ./node_modules/.bin/remix-serve ./build/index.js"]
Copy link
Member

@raducristianpopa raducristianpopa Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the ENABLE_INSECURE_MESSAGE_COOKIE should be passed as a build argument in the compose file. ENABLE_INSECURE_MESSAGE_COOKIE will be true when starting the frontend application every time (for everyone that is going to use Rafiki's frontend image), since we use this specific Dockerfile to build the image.

https://docs.docker.com/compose/compose-file/compose-file-v3/#args

Suggested change
CMD ["sh", "-c", "NODE_ENV=production ENABLE_INSECURE_MESSAGE_COOKIE=true ./node_modules/.bin/remix-serve ./build/index.js"]
ARG ENABLE_INSECURE_MESSAGE_COOKIE=false # default value if the argument is not present at build time
ENV NODE_ENV=production
ENV ENABLE_INSECURE_MESSAGE_COOKIE=$ENABLE_INSECURE_MESSAGE_COOKIE
CMD ["sh", "-c", "./node_modules/.bin/remix-serve ./build/index.js"]

I experimented with this method and it appears to be effective for me. I would greatly appreciate any additional feedback on it to see if it works for everyone since I'm not entirely sure about this approach.

Copy link
Member

@raducristianpopa raducristianpopa Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above approach might not even be necessary, we can just get rid of the inline environment variables and specify the value for ENABLE_INSECURE_MESSAGE_COOKIE in the compose files. Setting the value of the environment variable before a command, it will replace the value that was previously set in the compose file.

Suggested change
CMD ["sh", "-c", "NODE_ENV=production ENABLE_INSECURE_MESSAGE_COOKIE=true ./node_modules/.bin/remix-serve ./build/index.js"]
CMD ["sh", "-c", "./node_modules/.bin/remix-serve ./build/index.js"]

Copy link
Contributor Author

@BlairCurrey BlairCurrey Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - I will move it to the compose.

I can also remove NODE_ENV=production from the frontend dockerfile but not necessarily the MASE because those are set to development by compose. I take the point that we shouldn't set it here. Maybe I can remove NODE_ENV=development from the compose for the MASE.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the development NODE_ENV from the MASE's in the docker compose and removed the inline env var from the dockerfile. MASE's start fine and look normal - audited for any NODE_ENV checks in the MASE and there are none so there seems to be no problem with that. b35dc09

8 changes: 4 additions & 4 deletions packages/frontend/app/lib/message.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { type Session, type SessionData, redirect } from '@remix-run/node'
import { createCookieSessionStorage } from '@remix-run/node'
import { parseBool } from '~/shared/utils'

const ONE_MINUTE_IN_S = 60

Expand All @@ -20,10 +21,9 @@ export const messageStorage = createCookieSessionStorage<
sameSite: 'lax',
maxAge: ONE_MINUTE_IN_S,
secrets: ['MY_SUPER_SECRET_TOKEN'],
secure:
process.env.COOKIE_SECURE === undefined
? true
: ['true', 't', '1'].includes(process.env.COOKIE_SECURE.toLowerCase())
secure: process.env.ENABLE_INSECURE_MESSAGE_COOKIE
? !parseBool(process.env.ENABLE_INSECURE_MESSAGE_COOKIE)
: process.env.NODE_ENV === 'production'
Comment on lines +24 to +26
Copy link
Contributor Author

@BlairCurrey BlairCurrey Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added so that we dont have to run in development mode to set this to insecure. Basically, you can pass in ENABLE_INSECURE_MESSAGE_COOKIE=true to set this to false otherwise it uses NODE_ENV like we currently do.

this means it's secure by default for production use and when running in dev mode we dont need to set ENABLE_INSECURE_MESSAGE_COOKIE

for good measure here's a state table:

ENABLE_INSECURE_MESSAGE_COOKIE NODE_ENV Secure
true any false
false any true
undefined 'production' true
undefined 'development' false
undefined undefined true

}
})

Expand Down
4 changes: 4 additions & 0 deletions packages/frontend/app/shared/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,7 @@ export const paymentSubpathByType: {
[PaymentType.Incoming]: 'incoming',
[PaymentType.Outgoing]: 'outgoing'
}

export const parseBool = (str: string) => {
return ['true', 't', '1'].includes(str.toLowerCase())
}
4 changes: 2 additions & 2 deletions packages/frontend/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
"sideEffects": false,
"scripts": {
"build": "remix build",
"dev": "PORT=3005 COOKIE_SECURE=false remix dev",
"dev": "PORT=3005 remix dev",
"lint:fix": "eslint --fix app/",
"lint:check": "eslint app/",
"start": "COOKIE_SECURE=false remix-serve ./build/index.js",
"start": "ENABLE_INSECURE_MESSAGE_COOKIE=true remix-serve ./build/index.js",
"typecheck": "tsc"
},
"dependencies": {
Expand Down