-
Notifications
You must be signed in to change notification settings - Fork 375
Convert demo application to Vite #10510
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
Conversation
| { | ||
| "files": ["packages/react-integration/demo-app-ts/**/*"], | ||
| "rules": { | ||
| "patternfly-react/no-anonymous-functions": "off", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This linting rule gives false positives for class components that already have a displayName, so I've turned this off. For example:
class MyComponent extends React.Component {
static displayName = "MyComponent"; // It doesn't like this.
}| "files": ["packages/react-integration/demo-app-ts/**/*"], | ||
| "rules": { | ||
| "patternfly-react/no-anonymous-functions": "off", | ||
| "react/react-in-jsx-scope": "off", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need for this, as Vite uses the JSX transform.
| "rules": { | ||
| "patternfly-react/no-anonymous-functions": "off", | ||
| "react/react-in-jsx-scope": "off", | ||
| "spaced-comment": "off" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gives a false positive, so I've disabled it for now.
| "start": "yarn build && concurrently --kill-others \"yarn watch\" \"yarn workspace @patternfly/react-docs develop\"", | ||
| "start:cypress": "lerna run cypress:open", | ||
| "start:demo-app": "lerna run start:demo-app --stream", | ||
| "start:demo-app:hot": "lerna run start:demo-app:hot --stream", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hot reloading is default in Vite during development, so it is no longer needed to keep this separate script around.
|
Preview: https://patternfly-react-pr-10510.surge.sh A11y report: https://patternfly-react-pr-10510-a11y.surge.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test never ran, so I removed it.
Signed-off-by: Jon Koops <jonkoops@gmail.com>
tlabaj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Replaces WebPack with Vite to build the demo app, this reduces the amount of dependencies and is also a faster and more consistent development experience. Works towards closing #7287