Skip to content

Conversation

@jcaianirh
Copy link
Member

@jcaianirh jcaianirh commented Aug 8, 2022

In the future, PF will remove the react-measure package as it isn't compatible with react 18. This will cause errors on our end because we relied on their react-measure type dependency. Added that dependency into our package.json as well as our dynamic plugin package.json. Created a story to follow pf and move to react-resize-detector as tech debt before moving to react 18:
https://issues.redhat.com/browse/CONSOLE-3247

@openshift-ci openshift-ci bot requested review from dtaylor113 and zherman0 August 8, 2022 16:33
@jcaianirh
Copy link
Member Author

@jerolimov made some react-measure updates to compensate for pf removing it

@jcaianirh
Copy link
Member Author

QE Approver:
/assign @yapei

Docs Approver:
/assign @opayne1

PX Approver:
/assign @RickJWagner

Console Approver:
/assign @jhadvig

LGTM Approver:
/assign @sg00dwin

"@patternfly/react-core": "4.162.2",
"@patternfly/react-table": "4.31.7",
"@types/react": "16.8.13",
"@types/react-measure": "^2.0.6",
Copy link
Member

@christoph-jerolimov christoph-jerolimov Aug 8, 2022

Choose a reason for hiding this comment

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

I understand that this is needed in our main package.json, but is this really required in the dynamic-demo-plugin? We don't use or export react-measure here, or?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jerolimov it is required based off the output of the build. According to the build output (build.sh)

`✨ Done in 330.80s.
~/src/github.com/openshift/console
~/src/github.com/openshift/console/dynamic-demo-plugin ~/src/github.com/openshift/console
yarn install v1.22.15
[1/4] 🔍 Resolving packages...
[2/4] 🚚 Fetching packages...
[3/4] 🔗 Linking dependencies...
warning "@openshift-console/dynamic-plugin-sdk > @patternfly/quickstarts@2.0.1" has unmet peer dependency "showdown@>=1.8.6".
warning " > react-i18next@11.12.0" has unmet peer dependency "i18next@>= 19.0.0".
warning " > file-loader@1.1.11" has incorrect peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".
[4/4] 🔨 Building fresh packages...
success Saved lockfile.
✨ Done in 12.46s.
yarn run v1.22.15
$ yarn clean && NODE_ENV=production yarn ts-node node_modules/.bin/webpack
$ rm -rf dist
$ ts-node -O '{"module":"commonjs"}' node_modules/.bin/webpack
assets by status 87.3 KiB [cached] 22 assets
orphan modules 1.79 MiB [orphan] 1735 modules
runtime modules 18.3 KiB 12 modules
built modules 169 KiB (javascript) 294 bytes (consume-shared) [built]
cacheable modules 169 KiB
modules by path ./ 46.3 KiB
modules by path ./components/ 36.4 KiB 12 modules
modules by path ./utils/ 9.89 KiB 6 modules
modules by path ../node_modules/ 123 KiB 7 modules
consume-shared-module modules 294 bytes
modules by path *.1 (singleton) 84 bytes 2 modules
modules by path *.0 (singleton) 84 bytes 2 modules
+ 3 modules
container entry 42 bytes [built] [code generated]

ERROR in /Users/jcaiani/src/github.com/openshift/console/dynamic-demo-plugin/tsconfig.json
/Users/jcaiani/src/github.com/openshift/console/dynamic-demo-plugin/tsconfig.json
[tsl] ERROR
TS2688: Cannot find type definition file for 'react-measure'.
The file is in the program because:
Entry point for implicit type library 'react-measure'
ts-loader-default_9d8b55b1b989782b

webpack 5.73.0 compiled with 1 error in 9984 ms
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.`

Choose a reason for hiding this comment

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

Interesting, thanks

@jcaianirh
Copy link
Member Author

/retest

@yanpzhan
Copy link
Contributor

yanpzhan commented Aug 9, 2022

@jcaianirh Hi, I launched a cluster against the pr, did some regression test on console pages, such as catalog page/tables/metrics/charts/topology and so on. Is it enough? if there is special checkpoint need to be tested?

@opayne1
Copy link
Contributor

opayne1 commented Aug 9, 2022

/label docs-approved

@openshift-ci openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Aug 9, 2022
@jcaianirh
Copy link
Member Author

/retest

@RickJWagner
Copy link

/label px-approved

@openshift-ci openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Aug 9, 2022
@RickJWagner RickJWagner removed their assignment Aug 9, 2022
@yanpzhan
Copy link
Contributor

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Aug 10, 2022
@jcaianirh
Copy link
Member Author

/retest

@zherman0
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 15, 2022
@rhamilto
Copy link
Member

/label tide/merge-method-squash

/approve

/lgtm

@openshift-ci openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Aug 22, 2022
@rhamilto
Copy link
Member

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 22, 2022
@rhamilto
Copy link
Member

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 22, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcaianirh, rhamilto, zherman0

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 23, 2022

@jcaianirh: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit 9fb8273 into openshift:master Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.