search

sveltejs / eslint-plugin-svelte

ESLint plugin for Svelte using AST
https://sveltejs.github.io/eslint-plugin-svelte/
MIT License
282 stars 30 forks source link

Change `allowReferrer` to `requireNoopener` #571

Closed benmccann closed 6 months ago

benmccann commented 11 months ago

Before You File a Bug Report Please Confirm You Have Done The Following...

What version of ESLint are you using?

any

What version of eslint-plugin-svelte are you using?

any

What did you do?

https://github.com/melt-ui/melt-ui/pull/443#discussion_r1302364342

rel="noreferrer"

What did you expect to happen?

rel="noreferrer" should not warn unless using svelte's legacy compiler option. We need a way to configure that

https://github.com/sveltejs/svelte/pull/8230

What actually happened?

warned

Link to GitHub Repo with Minimal Reproducible Example

simply add rel="noreferrer". if you need a reproduction I can provide

Additional comments

No response

ota-meshi commented 11 months ago

It looks to me like you're using the svelte/no-target-blank rule yourself. Turn off rules when not needed 😉

https://sveltejs.github.io/eslint-plugin-svelte/rules/no-target-blank/

benmccann commented 11 months ago

oh, you are right! i'm sorry for this report. thank you so much for the help

benmccann commented 11 months ago

Thanks for the earlier advice. We tried to configure https://sveltejs.github.io/eslint-plugin-svelte/rules/no-target-blank/ appropriately, but were unable to do so (https://github.com/melt-ui/melt-ui/pull/443#discussion_r1303815634). I think the allowReferrer option isn't exactly correct because it allows you to write rel="noopener" when I think what you need to write is rel="noreferrer". We should probably replace the allowReferrer option with a requireNoopener option

ota-meshi commented 11 months ago

For clarity, could you please provide code examples of how the new option works and the correct and incorrect code when using the option?

By the way, the rule option is based on rule implemented for JSX.

https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/jsx-no-target-blank.md

github-actions[bot] commented 8 months ago

This issue is is stale because it missing information and has been open for 60 days with no activity.

mcmxcdev commented 8 months ago

Still relevant

github-actions[bot] commented 6 months ago

This issue is is stale because it missing information and has been open for 60 days with no activity.

mcmxcdev commented 6 months ago

Still relevant

JounQin commented 6 months ago

@mcmxcdev Please help to answer @ota-meshi's question instead of posting meaningless comments?

mcmxcdev commented 6 months ago

The rule currently requires the user to write rel="noreferrer noopener" although only rel="noreferrer" should be necessary. That would mean that the allowReferrer option (false by default) should actually be called requireNoopener and be true by default.

You can find additional context on https://github.com/melt-ui/melt-ui/pull/443#discussion_r1302364342 and https://github.com/sveltejs/svelte/pull/6289#issuecomment-831508775

benmccann commented 6 months ago

Sorry for missing that there was a question here.

I just spent the past hour and a half reading though a bunch of related issues on this to referesh my memory of this issue and provide clarification and realized there are no code changes required here and that my request was driven by a pair of misunderstandings, so I will close this issue. I do think some doc updates would be very helpful as I really badly misunderstood this rule in multiple ways and it seems likely that @mcmxcdev had some misunderstanding as well, so I sent https://github.com/sveltejs/eslint-plugin-svelte/pull/656 to address that.

Thank you for maintaining this great plugin!