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

Add browserslist-lint results #455

Merged
merged 10 commits into from
Aug 23, 2022
Merged

Add browserslist-lint results #455

merged 10 commits into from
Aug 23, 2022

Conversation

sashachabin
Copy link
Collaborator

@sashachabin sashachabin commented Aug 22, 2022

Changes

  • Added lint: [ { type, message }, ... ] field to HTTP API response
    UPD: it seems the server code is not updated in the preview
  • Added output in the client. Create warning, similar errors
  • Fixed Node.js examples lint error (Add ignoring Node.js in countryWasIgnored rule lint#1)
  • Updated README.md

Questions

  • How to display multiple warnings? (design) Now I output with . <br />
    image
  • Should we do something for warning's a11y?
  • Should we automatically update browserslist-lint every day (as browserslist and caniuse-lite)?
  • Our first example with defaults and supports es6-module contains coverage errors :(

Closes #390

@sashachabin sashachabin requested review from ai and ariarzer August 22, 2022 21:07
@sashachabin
Copy link
Collaborator Author

sashachabin commented Aug 22, 2022

Package size limit has exceeded by 286 B

I can combine renderWarning and renderError functions

@sashachabin
Copy link
Collaborator Author

image

Now we can use this design.
But alerts will have problems with layout shift :(

@ai
Copy link
Member

ai commented Aug 22, 2022

How to display multiple warnings? (design) Now I output with .

It is better to add new design.

Should we do something for warning's a11y?

We will do it as a part of #442

Should we automatically update browserslist-lint every day (as browserslist and caniuse-lite)?

Good idea. Done a0db083

Our first example with defaults and supports es6-module contains coverage errors :(

Yes, we will think about it in next major update of Browserslist

@ai
Copy link
Member

ai commented Aug 22, 2022

Note that right now there is an error on result rendering

TypeError: o is undefined

@sashachabin
Copy link
Collaborator Author

sashachabin commented Aug 23, 2022

Note that right now there is an error on result rendering

Yes. I haven't figured out what the problem is yet.

I wrote about it above.
Locally there is a lint field in HTTP API response, but it is not in the preview.

@ai
Copy link
Member

ai commented Aug 23, 2022

Why is not updated? We deploy client and server together

@sashachabin
Copy link
Collaborator Author

sashachabin commented Aug 23, 2022

Why is not updated? We deploy client and server together

Fixed! You can watch the preview :)

I forgot about npm link when I edited browserslist-lint yesterday. Everything is fine with the deployment)

@ai
Copy link
Member

ai commented Aug 23, 2022

We need to remove "" from warnings or replace it to ("" was used to highlight important parts of the warning)

Снимок экрана от 2022-08-23 10-17-21

@ai
Copy link
Member

ai commented Aug 23, 2022

I moved full design to another task #457

@sashachabin
Copy link
Collaborator Author

sashachabin commented Aug 23, 2022

Replaced quotes with <strong>...</strong>. Looks good

image

@sashachabin sashachabin marked this pull request as ready for review August 23, 2022 09:50
@sashachabin sashachabin merged commit c6a55dc into main Aug 23, 2022
@sashachabin sashachabin deleted the add-lint branch August 23, 2022 10:26
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.

Add browserslist-lint results
2 participants