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

chore(DashboardWrapper + DashboardHeader) convert to TS #9618

Merged
merged 8 commits into from
Oct 6, 2023

Conversation

Dominik-Petrik
Copy link
Contributor

@Dominik-Petrik Dominik-Petrik commented Sep 13, 2023

What:

When yarn start is run there are a few module resolution changes I am not able to solve.

@patternfly-build
Copy link
Contributor

patternfly-build commented Sep 13, 2023

@jenny-s51 jenny-s51 force-pushed the dashboardWrapper-convertToTS branch 2 times, most recently from f90c511 to 434ac28 Compare September 19, 2023 20:45
Copy link
Contributor

@jenny-s51 jenny-s51 left a comment

Choose a reason for hiding this comment

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

Fixed all module resolution errors and this is working and building as expected locally - currently investigating rollup issue in build:umd step in CI.

I've moved the DashboardWrapper and DashboardHeader components into react-core since it makes more sense to import them into files with the other react-core imports. This is going to need some build/infra related updates.

chore(DashboardWrapper) convert to TS

move to utils and update imports

fix md line

fix build errors

update columnmanagement dashboardwrapper import

testing move files  out of utils

change to absolute imports

change to relative imports for each component

update page import

add rollup plugin-typescript

rename rollup.config.js to rollup.config.mjs

revert build changes and remove imports from top of md files

revert yarn.lock, revert to index.d.ts, update jumplinks import
Copy link
Contributor

@jenny-s51 jenny-s51 left a comment

Choose a reason for hiding this comment

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

Update: Added rollup/plugin-svg which was missing from configs and causing svg imports to break the build. This was breaking jest tests as well which needed their own svg transformer jest-transform-stub.

These updates to the build here should also close patternfly/patternfly-org#3757 in org.

Note: merging this should unblock a lot of the remaining work/PRs in the demo conversion epic , several of which depended on the Dashboard components getting converted to TS

@jenny-s51 jenny-s51 changed the title chore(DashboardWrapper) convert to TS chore(DashboardWrapper + DashboardHeader) convert to TS Sep 22, 2023
Copy link
Contributor

@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

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

Looks like there's a few Table demos still importing the old DashboardWrapper from the demos/examples directory (Empty, Loading, and Error in the "Empty states" section of examples). Otherwise looks good. Only thing is if we'd want to have the Dashboard components able to be imported by consumers; there'd probably be not much use-case for them to import it in the first place, but just wondering if the files should remain in the demos directory, or just not exported in our index file.

Copy link
Contributor

@jenny-s51 jenny-s51 left a comment

Choose a reason for hiding this comment

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

Thank you @thatblindgeye - good point. At the moment our build-configs don't seem to support TSX exports from demos/examples, so I'm investigating how we can make the necessary updates to move these Dashboard components back to the demos folder.

@jenny-s51 jenny-s51 requested a review from tlabaj October 6, 2023 13:28
Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

@tlabaj tlabaj merged commit 93ea576 into patternfly:main Oct 6, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert DashboardWrapper to TS Convert DashboardHeader to TS
5 participants