Skip to content

fix(reserved-member-names): improve reserved member check by using HTMLElement#101

Merged
raphaelpor merged 3 commits intostenciljs:mainfrom
jcfranco:jcfranco/100-improve-HTMLElement-reserved-member-check
Mar 4, 2024
Merged

fix(reserved-member-names): improve reserved member check by using HTMLElement#101
raphaelpor merged 3 commits intostenciljs:mainfrom
jcfranco:jcfranco/100-improve-HTMLElement-reserved-member-check

Conversation

@jcfranco
Copy link
Copy Markdown
Contributor

@jcfranco jcfranco commented Dec 20, 2023

Related issue: #100

This PR aims to make the check for reserved member names more robust by creating list of reserved attributes/props from a local custom element (HTMLElement instance).

Additional changes

@jcfranco jcfranco changed the title fix(reserved-member-names): improve reserved member check by testing against HTMLElement fix(reserved-member-names): improve reserved member check by using HTMLElement Dec 21, 2023
@James-Wilkinson-git
Copy link
Copy Markdown

What does this produce as a result of what is reserved? It should match https://www.w3schools.com/tags/ref_attributes.asp all of these different types of HTML components attribute are reserved if you use something like a web component in svelte

@jcfranco
Copy link
Copy Markdown
Contributor Author

jcfranco commented Feb 9, 2024

It should warn against property/method names that match global attributes and properties found on HTMLElement (see #100 (comment) for additional context).

@James-Wilkinson-git
Copy link
Copy Markdown

This change will reduce the currently erroring names, this would break backwards compatibility for other users and ads I’ve stated in the linked issue we should actually be expanding this list to match all reserved html attributes of all elements.

@jcfranco
Copy link
Copy Markdown
Contributor Author

jcfranco commented Feb 9, 2024

The rule's documentation specifies it targets reserved global HTML attribute names for props and methods. My PR addresses false positives for non-global attributes like color, aligning with the rule's intent. Framework-specific quirks are outside the rule's scope.

@James-Wilkinson-git
Copy link
Copy Markdown

James-Wilkinson-git commented Feb 9, 2024

I can side with that but people already using the linter rule will now no longer have a guard so it would be a breaking change that should come with another option to replace what is being removed.

@jcfranco
Copy link
Copy Markdown
Contributor Author

jcfranco commented Feb 9, 2024

I don't agree it being a breaking change from the documentation perspective. The current behavior, which flags additional, unrelated properties, should be considered a bug.

In any case, could you provide a snippet or example use case along with the error/warning? I'm unfamiliar with Svelte and really trying to understand the issue. I don't get why Svelte would complain about the attribute/prop name of a 3rd-party custom element.

@jcfranco
Copy link
Copy Markdown
Contributor Author

jcfranco commented Mar 4, 2024

@raphaelpor Are there any additional steps needed from me to get this landed? This PR was approved a while back and I'm eager to help it progress. Thanks!

@raphaelpor raphaelpor merged commit 483dd53 into stenciljs:main Mar 4, 2024
@jcfranco
Copy link
Copy Markdown
Contributor Author

jcfranco commented Mar 4, 2024

@raphaelpor Thanks for the merge! 🚀

@jcfranco jcfranco deleted the jcfranco/100-improve-HTMLElement-reserved-member-check branch March 5, 2024 01:04
jcfranco referenced this pull request in Esri/calcite-design-system Apr 3, 2024
….7.2 (#9036)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[@stencil-community/eslint-plugin](https://togithub.com/stencil-community/stencil-eslint)
| [`0.7.1` ->
`0.7.2`](https://renovatebot.com/diffs/npm/@stencil-community%2feslint-plugin/0.7.1/0.7.2)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@stencil-community%2feslint-plugin/0.7.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@stencil-community%2feslint-plugin/0.7.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@stencil-community%2feslint-plugin/0.7.1/0.7.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@stencil-community%2feslint-plugin/0.7.1/0.7.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>stencil-community/stencil-eslint
(@&#8203;stencil-community/eslint-plugin)</summary>

###
[`v0.7.2`](https://togithub.com/stencil-community/stencil-eslint/releases/tag/v0.7.2)

[Compare
Source](https://togithub.com/stencil-community/stencil-eslint/compare/v0.7.1...v0.7.2)

#### What's Changed

- Added fixes to append public or private keywords by
[@&#8203;limxingzhi](https://togithub.com/limxingzhi) in
[https://github.com/stencil-community/stencil-eslint/pull/23](https://togithub.com/stencil-community/stencil-eslint/pull/23)
- fix(reserved-member-names): improve reserved member check by using
`HTMLElement` by [@&#8203;jcfranco](https://togithub.com/jcfranco) in
[https://github.com/stencil-community/stencil-eslint/pull/101](https://togithub.com/stencil-community/stencil-eslint/pull/101)

#### New Contributors

- [@&#8203;limxingzhi](https://togithub.com/limxingzhi) made their first
contribution in
[https://github.com/stencil-community/stencil-eslint/pull/23](https://togithub.com/stencil-community/stencil-eslint/pull/23)
- [@&#8203;jcfranco](https://togithub.com/jcfranco) made their first
contribution in
[https://github.com/stencil-community/stencil-eslint/pull/101](https://togithub.com/stencil-community/stencil-eslint/pull/101)

**Full Changelog**:
stenciljs/eslint-plugin@v0.7.1...v0.7.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 5am every weekday" in timezone
America/Los_Angeles, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/Esri/calcite-design-system).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNjkuMiIsInVwZGF0ZWRJblZlciI6IjM3LjI2OS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

bug: props unrelated to HTMLElement are being flagged by reserved-member-names rule

3 participants