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

Upgrade react-scripts to 4.0.0 #269

Merged
merged 1 commit into from
Dec 4, 2020
Merged

Conversation

ikornienko
Copy link
Contributor

This is an incremental continuation of effort from #216.

Things to note:

  • couldn't go to [email protected] because of .eslintcache constantly generated facebook/create-react-app#10161 as VSCode isn't yet friends with ESLint caching feature;
  • added postcss as a dev dependency as it's not a dependency of postcss-cli since 8.x;
  • bunch of linting fixes, mostly being more strict with types and avoiding any;
  • added assertDefined utility function as an escape hatch against not strict swagger definitions that don't specify that, say, ID is not an optional property and always present... not ideal, yet asserting on something that MUST be true feels conceptually right.

Major changes:
 - added postcss as it's not a dependency of postcss-cli since 8.x
 - bunch of linting fixes, mostly being more strict with types and avoiding 'any'
 - added 'assertDefined' utility function as an escape hatch against not strict swagger definitions that don't specify that, say, 'ID' is not an optional property and always present
@@ -13,18 +13,40 @@ type Props = {
onModify?: (notation: string, incOrDec: string) => void;
};

export default function Countdown({ targetDate, className = '', onModify }: Props): ReactElement {
const duration = calcDuration(targetDate);
type ModifiableTimeUnitProps = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this file nothing changed except of the order in which components are defined. Airbnb code style now requires things to be defined before they're used, which is a common practice I've seen across many respectful OSS projects. It's an opinionated thing... yet we agreed before to outsource decisions about opinionated topics to Prettier and Airbnb, so following the suite :)

};

export default function DownloadArtifactsModal({ cluster, onClose }: Props): ReactElement {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here, it's just re-ordering of component definitions.

Copy link
Contributor

@gavin-stackrox gavin-stackrox left a comment

Choose a reason for hiding this comment

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

👍 for assertDefined.

@ikornienko ikornienko merged commit 9e5eb76 into master Dec 4, 2020
@ikornienko ikornienko deleted the ivan/upgrade-react-scripts-to-4 branch December 4, 2020 18:39
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.

2 participants