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

Jump to bottom

feat(dev): mock API server with miragejs #1039

Merged
merged 1 commit into from
Jun 2, 2023
Merged

feat(dev): mock API server with miragejs #1039

merged 1 commit into from
Jun 2, 2023

Conversation

tthvo
Copy link
Member

@tthvo tthvo commented May 31, 2023 •
edited
Loading

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed the last commit: git commit --amend --signoff

Fixes: #1026

Description of the change:

Use Mirage JS to mock API server and web socket for cases where a quick preview is needed. This also includes 2 yarn scripts to start hot-reload server and build asset with these mocks injected.

Mirage JS is implemented in TypeScript to allow static type checking. A little more work to set up but seems to work fine.

To support intergration with Mirage JS, the following refactoring is needed:

  • Services cannot initialize within constructor -> Added an init function to be called before app is built and after mirage is started.
  • Remove deps on state in useEffect. This sometimes causes infinite setStates when using mocks.
  • Remove dotenv webpack plugin and directly use EnvironmentPlugin so we don't have to create a new .env for preview.

Other changes:

Motivation for the change:

See #1026

How to manually test:

To start development server with API server mocked:

yarn start:dev:preview

To build assets with mocks injected

yarn build:preview:notests

To quickly preview built web assets, use

cd dist && npx browser-sync start --server

@tthvo tthvo added the feat New feature or request label May 31, 2023
@github-actions github-actions bot added the needs-triage Needs thorough attention from code reviewers label May 31, 2023
@mergify mergify bot added the safe-to-test label May 31, 2023
@tthvo tthvo removed the needs-triage Needs thorough attention from code reviewers label May 31, 2023
@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1039-fa6cdbd775a8b927c9df30998861b93b32a9062b sh smoketest.sh

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1039-488b8723eb9efd00a9be648e3c94b38112ee4ff5 sh smoketest.sh

@tthvo tthvo force-pushed the live-demo branch 4 times, most recently from 135609b to 7c51de4 Compare May 31, 2023 09:41
@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1039-7c51de428a6aa30d77048bb33791f9094113cc95 sh smoketest.sh

@tthvo tthvo force-pushed the live-demo branch 2 times, most recently from 3cbadd1 to 591a662 Compare June 1, 2023 06:42
@github-actions
Copy link

github-actions bot commented Jun 1, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1039-591a6621ec2597e443b7f1f381ece9b7f209761b sh smoketest.sh

@github-actions
Copy link

github-actions bot commented Jun 1, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1039-a4b1adfc0c14f54afe7dfa59ac88bb7c15a42b82 sh smoketest.sh

@tthvo tthvo marked this pull request as ready for review June 1, 2023 08:00
@tthvo tthvo requested review from andrewazores and ebaron as code owners June 1, 2023 08:00
@tthvo
Copy link
Member Author

tthvo commented Jun 1, 2023 •
edited
Loading

Ready for review now!

Mirage code is tree-shaken out of the production bundle with sideEffects: false when using yarn build:notests:

  • Reference: https://webpack.js.org/guides/tree-shaking/
  • I got some issues with production css order when mirage code imports internal types (i.e. Target, Rule, ActiveRecording). So, for now, I just type as any to bypass static type checking. This does not occur in development build.
  • We probably want to move all common types into a separate src/app/typings.ts so it can be used for mirage. Maybe another follow-up PR then?

@github-actions
Copy link

github-actions bot commented Jun 1, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1039-c29e52d6e41024fdfbfc0f7432204f2c4d832f38 sh smoketest.sh

andrewazores
andrewazores reviewed Jun 1, 2023
View reviewed changes
src/index.tsx Outdated Show resolved Hide resolved
andrewazores
andrewazores reviewed Jun 1, 2023
View reviewed changes
src/index.tsx Outdated Show resolved Hide resolved
@tthvo
Copy link
Member Author

tthvo commented Jun 1, 2023 •
edited
Loading

Updated to split mirage into a separate chunk. Its not obvious that the chunk is executed before app*.js but seems to work and minimize changes.

@github-actions
Copy link

github-actions bot commented Jun 1, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1039-1f857247140d201c85485e83959a80444fa0b96f sh smoketest.sh

@andrewazores
Copy link
Member

Sweet, this is looking really good.

Are any of the changes in the application bundle still needed for the Mirage shim to work? Or is that just remaining cleanup that was done along the way in the first approach?

@andrewazores
Copy link
Member

I just tested it out with the two manual test modes you gave and both look great.

If the other refactoring/cleanup changes I mentioned in the last comment aren't required for this PR, please split them out into a separate one. I'd just like to keep the history separated, just in case it turns out that this causes some problems later, it's easier to maintain and back this out or carry it as a separate patch set etc.

@tthvo
Copy link
Member Author

tthvo commented Jun 2, 2023 •
edited
Loading

Opps sorry. Somehow github notifications just didn't show up in my inbox :D Weird... Anyways,

Are any of the changes in the application bundle still needed for the Mirage shim to work? Or is that just remaining cleanup that was done along the way in the first approach?

These changes are just eslint fixes and refactoring. I will open a new PR.

feat(dev): mock API server with miragejs
e0c79d5 
Signed-off-by: Thuan Vo <thvo@redhat.com>
@tthvo
Copy link
Member Author

tthvo commented Jun 2, 2023

Updated to remove eslint fixes and refactoring now :))

Changes required for integrating MirageJS is included along side with some webpack production build optimization:

  • Remove comments from bundle and css.
  • Remove source maps (only enabled for development mode).

@github-actions
Copy link

github-actions bot commented Jun 2, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1039-e0c79d5c8062f7e9548f70a965bb60331f1cb0fe sh smoketest.sh

andrewazores
andrewazores approved these changes Jun 2, 2023
View reviewed changes
Copy link
Member

@andrewazores andrewazores left a comment

Choose a reason for hiding this comment

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

Very nice, thank you.

@andrewazores andrewazores merged commit b9d9557 into cryostatio:main Jun 2, 2023
@tthvo tthvo mentioned this pull request Jun 6, 2023
Merged
7 tasks
@tthvo tthvo deleted the live-demo branch June 27, 2023 08:51
@tthvo tthvo mentioned this pull request Aug 12, 2023
Merged
7 tasks
@tthvo tthvo mentioned this pull request Aug 22, 2023
Merged
7 tasks
@andrewazores andrewazores mentioned this pull request Aug 30, 2023
Merged
7 tasks
mergify bot pushed a commit that referenced this pull request Aug 31, 2023
@mergify
feat(dev): mock API server with miragejs (#1039)
05efc24 
Signed-off-by: Thuan Vo <thvo@redhat.com>
(cherry picked from commit b9d9557)
@mergify mergify bot mentioned this pull request Aug 31, 2023
Merged
andrewazores pushed a commit that referenced this pull request Aug 31, 2023
@mergify
feat(dev): mock API server with miragejs (#1039) (#1098)
b7df7ce 
Signed-off-by: Thuan Vo <thvo@redhat.com>
(cherry picked from commit b9d9557)

Co-authored-by: Thuan Vo <thvo@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewazores andrewazores andrewazores approved these changes

@ebaron ebaron Awaiting requested review from ebaron

Assignees
No one assigned
Labels
backport feat New feature or request safe-to-test
Projects
No open projects
2.3.1 release
Status: Done
2.4.0 release
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Story] Mock API server for rapid frontend development
2 participants
@tthvo @andrewazores