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

[WIP] Update: add htmlhint #758

Closed
wants to merge 1 commit into from
Closed

[WIP] Update: add htmlhint #758

wants to merge 1 commit into from

Conversation

aladdin-add
Copy link
Contributor

fixes #210

@maxmilton
Copy link

maxmilton commented Jun 23, 2017

Should we provide a .htmlhintrc file with some preset defaults too?

Edit: Just read https://github.com/yaniswang/HTMLHint/wiki/Usage#how-to-use and saw if .htmlhintrc is missing it will use a default set of rules:

{
    "tagname-lowercase": true,
    "attr-lowercase": true,
    "attr-value-double-quotes": true,
    "doctype-first": true,
    "tag-pair": true,
    "spec-char-escape": true,
    "id-unique": true,
    "src-not-empty": true,
    "attr-no-duplication": true,
    "title-require": true
}

Seems reasonable but maybe we should have a comment with a link to that page? Or to: https://github.com/yaniswang/HTMLHint/wiki/Rules

@maxmilton
Copy link

maxmilton commented Jun 23, 2017

Also, should this be an option together with ESLint or should it be a separate option during vue-cli init time? I'm not sure which is better to be honest. Thoughts anyone?

@nickmessing
Copy link

@aladdin-add, @maxmilton, I think having a separate option would be better than combining it with eslint since the whole vue ecosystem is non-opinionated and highly configurable, even tho I think nobody will use just one of them.

enforce: 'pre',
loader: 'htmlhint-loader',
exclude: /node_modules/
},

Choose a reason for hiding this comment

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

I think this should be wrapped in {{#lint}}...{{/lint}} just like eslint-loader or have another key in meta.js something like htmlLint

@@ -37,6 +37,7 @@
"eslint-friendly-formatter": "^2.0.7",
"eslint-loader": "^1.7.1",
"eslint-plugin-html": "^2.0.0",
"htmlhint-loader": "1.3.0",

Choose a reason for hiding this comment

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

Same as above comment

@nickmessing
Copy link

I think, just like @maxmilton said, there should be .htmlhintrc and it would be nice to add htmllint src/**/*.vue to scripts in package.json I think.

@aladdin-add aladdin-add changed the title Update: add htmlhint [WIP] Update: add htmlhint Jun 23, 2017
@aladdin-add
Copy link
Contributor Author

I got an error running npm run dev, after adding this loader.
image

there are two issues here:

  • showing "Failed to compile with 1 errors", it is no much helpful to a user. can we change some config to show more info?
  • why this error? I just followed the htmlhint-loader's docs, did I forget something?

@maxmilton
Copy link

Looks like there might soon be a solution for the blank error in friendly-errors-webpack-plugin, see this PR: geowarin/friendly-errors-webpack-plugin#52

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.

None yet

3 participants