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(@aws-amplify/ui-components): Local prettier run on ui-components #8136

Merged
merged 5 commits into from
Apr 23, 2021

Conversation

wlee221
Copy link
Contributor

@wlee221 wlee221 commented Apr 21, 2021

Description of changes

This is a follow-up to #7790, which consolidated prettierrc at the root. The prettier changes haven't been applied to ui-components files yet, so this pr runs prettier on them. Submitting this so that future UI Components PR are free of unrelated style changes.

Issue #, if available

Description of how you validated changes

yarn test and eslint passed. The only code change was disabling an eslint rule that forces decorators to be inline with the variable. See comment below.

Checklist

  • PR description included
  • yarn test passes

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-commenter
Copy link

codecov-commenter commented Apr 21, 2021

Codecov Report

Merging #8136 (ddd82b8) into main (28e89f4) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #8136   +/-   ##
=======================================
  Coverage   75.58%   75.58%           
=======================================
  Files         215      215           
  Lines       13592    13592           
  Branches     2676     2676           
=======================================
  Hits        10273    10273           
  Misses       3117     3117           
  Partials      202      202           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28e89f4...ddd82b8. Read the comment docs.

'@typescript-eslint/no-empty-function': 0,
'@typescript-eslint/ban-ts-comment': 0,
'@typescript-eslint/ban-types': 0,
'@stencil/decorators-style': 0
Copy link
Contributor Author

@wlee221 wlee221 Apr 21, 2021

Choose a reason for hiding this comment

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

This is the only code change made. With printwidth reduced from 120 to 80, decorators were moved to newlines which eslint didn't like. I disabled the rule here ('@stencil/decorators-style': 0).

@wlee221 wlee221 requested a review from sammartinez as a code owner April 21, 2021 21:36
@@ -47,7 +47,7 @@
},
{
"path": "dist/withSSRContext+Storage.js.min.js",
"maxSize": "181kB"
Copy link
Contributor Author

@wlee221 wlee221 Apr 21, 2021

Choose a reason for hiding this comment

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

One more file with actual code change! 😛 Bumping this to 190 kB as discussed internally.

@@ -43,11 +43,11 @@
},
{
"path": "dist/withSSRContext.js.min.js",
"maxSize": "146kB"
"maxSize": "155kB"
Copy link
Contributor Author

@wlee221 wlee221 Apr 21, 2021

Choose a reason for hiding this comment

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

Increasing this to 155kB as well (+9 kB), which have been failing recently as well

Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity was there a reason that we chose +9 kB and is that likely to need to change again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No particular reason for +9kB. But the goal is that this should be small enough to catch unusually high increase in bundle size. This will need to be changed again eventually as our bundle size creeps up little by little, which I'm okay with as long as we're capable of catching unusual bundle size increase.

@wlee221 wlee221 merged commit 5317524 into main Apr 23, 2021
@ErikCH ErikCH added UI Related to UI Components and removed Amplify UI Components labels May 19, 2021
@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 20, 2022
@jimblanc jimblanc deleted the ui-prettier-run branch November 23, 2022 15:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
UI Related to UI Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants