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 module as a type of <script> #95

Closed
wants to merge 1 commit into from
Closed

Conversation

yykamei
Copy link
Contributor

@yykamei yykamei commented Sep 9, 2022

Recently, JavaScript module would be getting used because almost all browsers support it, so module could also be added in VALID_JAVASCRIPT_TAG_TYPES as a type of <script>.

What do you think?

@yykamei
Copy link
Contributor Author

yykamei commented Sep 9, 2022

I signed in the CLA.

Copy link

@seanpdoyle seanpdoyle left a comment

Choose a reason for hiding this comment

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

Is it intentional that BetterHtml includes a set of acceptable values that is less strict than those declared by shopify/erb_lint? Is it intentional that they share overlapping responsibilities?

Is there an opportunity to replace one with the other?

If not, should ERBLint::Linters::AllowedScriptType be updated to include "module" as well?

@yykamei
Copy link
Contributor Author

yykamei commented Nov 19, 2022

Is it intentional that BetterHtml includes a set of acceptable values that is less strict than those declared by shopify/erb_lint? Is it intentional that they share overlapping responsibilities?

I think it's a good point, but I'm not sure the relationship between erb-lint and better-html.

Actually, I'm using erb-lint with ErbSafety enabled. As you said, I configured AllowedScriptType to allow "module", but still, I encountered a similar issue, and I found out the issue is raised from better-html. So far, erb-lint has no options to configure better-html. That's why I opened this PR.

linters:
  AllowedScriptType:
    enabled: true
    allowed_types:
      - 'application/json'
      - 'module'
  DeprecatedClasses:
    enabled: true
  ErbSafety:
    enabled: true

@rafaelfranca
Copy link
Member

better-html is a HTML parser that can be also used to transform unsafe HTML to safe HTML. We use it as a security measurement. It isn't linter. It just happen to be used the erb-lint as the parser for HTML and ERB.

The configuration of the linter should be independent from the list of allowed types in better-html. Their defaults can be the same though.

@rafaelfranca
Copy link
Member

Can you please rebase the PR so I can start the CI?

@yykamei
Copy link
Contributor Author

yykamei commented Dec 9, 2022

Thank you. I rebased it.

Recently, JavaScript module would be getting used because almost all
browsers support it, so `module` could also be added in
`VALID_JAVASCRIPT_TAG_TYPES` as a `type` of `<script>`.
@yykamei
Copy link
Contributor Author

yykamei commented Feb 28, 2023

I also rebased again.

rafaelfranca added a commit that referenced this pull request Apr 13, 2023
@rafaelfranca
Copy link
Member

Merged in de1fe4d

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