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.

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

fix(cards): replace instances of card.component.name #1081

Merged
merged 3 commits into from
Aug 5, 2023

Conversation

maxcao13
Copy link
Member

@maxcao13 maxcao13 commented Aug 3, 2023

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: #1079

Description of the change:

With help from Thuan, we figure out the bug. This change replaces all instances where we reference the Functional component name as data related to the dashboard. We need to do this because, it seems like Webpack minifys all instances of the component name in production mode: https://stackoverflow.com/questions/43800784/get-component-name-in-react. And then we end up with weird names and bad states because of this. That's also why there was no bug if running the yarn start:dev dev server.

How to manually test:

  1. CD to a cryostat repo.
  2. cd web-client and checkout this branch in that submodule e.g. git checkout f8592d891067c18a1f61f0cdbabc995974e56944
  3. cd back to .. and run mvn clean -DskipTests package
  4. Clear browser localstorage on https://localhost:8181
  5. Should be solved if creating a new layout template.

@github-actions
Copy link

github-actions bot commented Aug 3, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1081-8b4b28d35523f57e16ade89994affceaf21492f3 sh smoketest.sh

@github-actions
Copy link

github-actions bot commented Aug 3, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1081-89b8d24d9c193a931acecace3426b7b5a6354aea sh smoketest.sh

Copy link
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

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

Looks good to me! Fixed the issue!

@andrewazores
Copy link
Member

andrewazores commented Aug 4, 2023

The minification explanation makes a lot of sense. I just ran into this, too (before this PR):

image

@andrewazores
Copy link
Member

@aali309 and @mwangggg could you verify that this bug does not occur in the 2.3.0 released images? I checked yesterday and didn't seem to run into this issue. You can run that image to check the behaviour, as well as you can review the release branch source code and see if it looks like the same bug could be present.

@tthvo
Copy link
Member

tthvo commented Aug 4, 2023

I can confirm that 2.3.0 image does not run into this bug. Seems like in 2.3.0, these functions are not minified and was allowed to keep source name.

I believe since we remove source map in production build after:

b9d9557#diff-d3465f3a71d5cc473d32d3919bb34796e47f195e78b453d88c727c3f5ad80e6e

The production code is now fully minified as desired, and also causes this bug to show up.

Here is a quick inspection of the production build of app....bundle.js. Notice, how the second one is minified and first one is just like source code (i.e. compiled to JS).

2.3.0 Build

var MBeanMetricsChartCardDescriptor = {\n    featureLevel: Settings_service/* FeatureLevel.PRODUCTION */.Lu.PRODUCTION,\n    title: \'CHART_CARD.MBEAN_METRICS_CARD_TITLE\',\n    cardSizes: MBeanMetricsChartCardSizes,\n    description: \'CHART_CARD.MBEAN_METRICS_CARD_DESCRIPTION\',\n    descriptionFull: "CHART_CARD.MBEAN_METRICS_CARD_DESCRIPTION_FULL",\n    component: MBeanMetricsChartCard,

Latest build

rd={featureLevel:Jt.Lu.PRODUCTION,title:"CHART_CARD.MBEAN_METRICS_CARD_TITLE",cardSizes:nd,description:"CHART_CARD.MBEAN_METRICS_CARD_DESCRIPTION",descriptionFull:"CHART_CARD.MBEAN_METRICS_CARD_DESCRIPTION_FULL",component:td,propControls:[{name:"CHART_CARD.PROP_CONTROLS.PERFORMANCE_METRIC.NAME",key:"chartKind",values:Xu.map((function(e){return e.displayName})),defaultValue:Xu[0].displayName

@andrewazores andrewazores merged commit 52a0ef0 into cryostatio:main Aug 5, 2023
22 checks passed
@maxcao13 maxcao13 deleted the card-component-name-fix branch August 5, 2023 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] Unknown card type selection: MBeanMetricsChartCard
4 participants