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

Fix eslint parsing errors, remove eslint-plugin-html #20323

Merged
merged 6 commits into from
Jul 15, 2022

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Jul 11, 2022

Introduce a separate .eslintrc.yaml in the Vue components folder to selectively enable vue-eslint-parser there, so that the rest of the files can use eslint's core parser which can deal with hashbangs.

The fact that the eslint-disable comments worked in HTML was a unintended side-effect of the files being parsed via vue-eslint-parser, so I had to disable the parsing of these files in .eslintrc.yaml to make it work.

For more context see #20113 and linked issues.

@silverwind silverwind added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Jul 11, 2022
@silverwind
Copy link
Member Author

silverwind commented Jul 11, 2022

BTW, if eslint-plugin-html is proving to much of a pain because it can't work with the template syntax, we could remove it. There's not much JS in HTML anyways, so it's not much value lost by removing it.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 11, 2022
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 12, 2022
Introduce a separate .eslintrc in the Vue components folder to
selectively enable vue-eslint-parser there, so that the rest of the
files can use eslint's core parser which can deal with hashbangs.

The fact that the eslint-disable comments worked in HTML was a
unintended side-effect of the files being parsed via vue-eslint-parser,
so I had to disable the parsing of these files in .eslintrc.yaml to make
it work.
@silverwind
Copy link
Member Author

Decided to remove eslint-plugin-html as it causes more issues than it solves.

@wxiaoguang you probably want to review this.

@silverwind silverwind changed the title Fix eslint parsing errors Fix eslint parsing errors, remove eslint-plugin-html Jul 14, 2022
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

I think it's fine. The only flaw is that the scripts in tmpl do not get linted?

IIRC we worried about the scripts in tmpl before (eg: #19209)

I am fine to either, since now we have explicitly JS error prompt (#18971)

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 15, 2022
@silverwind
Copy link
Member Author

Yes, inline scripts won't get linted anymore, need to be review those extra-carefully.

@wxiaoguang wxiaoguang merged commit 4c0fce8 into go-gitea:main Jul 15, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 15, 2022
* upstream/main:
  Fix commit status icon when in subdirectory (go-gitea#20285)
  Fix eslint parsing errors, remove eslint-plugin-html (go-gitea#20323)
  Include login_name in adminCreateUser response (go-gitea#20283)
  Add allow_rebase_update, default_delete_branch_after_merge to repository api response (go-gitea#20079)
  Allow to specify colors for text in markup (go-gitea#20363)
  [skip ci] Updated translations via Crowdin
  update xorm.io/xorm v1.3.2-0.20220714055524-c3bce556200f (go-gitea#20371)
  Add order by for assignee no sort issue (go-gitea#20053)
  Make sure `repo_dir` is an empty directory or doesn't exist before 'dump-repo' (go-gitea#20205)
  Fix English mistakes in some Markdown documents (go-gitea#20274)
  Fix versions check for busybox `sh` (go-gitea#20358)
dineshsalunke pushed a commit to dineshsalunke/gitea that referenced this pull request Jul 15, 2022
Introduce a separate .eslintrc in the Vue components folder to
selectively enable vue-eslint-parser there, so that the rest of the
files can use eslint's core parser which can deal with hashbangs.

The fact that the eslint-disable comments worked in HTML was a
unintended side-effect of the files being parsed via vue-eslint-parser,
so I had to disable the parsing of these files in .eslintrc.yaml to make
it work, and finally decided to remove eslint-plugin-html as it causes
more issues than it solves.
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
Introduce a separate .eslintrc in the Vue components folder to
selectively enable vue-eslint-parser there, so that the rest of the
files can use eslint's core parser which can deal with hashbangs.

The fact that the eslint-disable comments worked in HTML was a
unintended side-effect of the files being parsed via vue-eslint-parser,
so I had to disable the parsing of these files in .eslintrc.yaml to make
it work, and finally decided to remove eslint-plugin-html as it causes
more issues than it solves.
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants