refactor: rename to blocklist/allowlist #690
Conversation
|
Thanks for your contribution. Could you rebase your PR? |
AndersDJohnson
force-pushed
the
blocklist
branch
from
1738c45 to
7a31e88
Compare
| @@ -232,7 +232,7 @@ const categories = { | |||
| { tag: 'escape', weight: Weight.MID }, | |||
| { tag: 'filter', weight: Weight.NORMAL }, | |||
| { tag: 'xss', weight: Weight.HIGH }, | |||
| { tag: 'whitelist', weight: Weight.SMALL }, | |||
| { tag: 'allowlist', weight: Weight.SMALL }, | |||
There was a problem hiding this comment.
Perhaps, unfortunately, this should remain tag: 'whitelist' if it refers to https://www.npmjs.com/search?q=whitelist
| whiteList: [], | ||
| blackList: [], | ||
| allowList: [], | ||
| blockList: [], |
There was a problem hiding this comment.
Are these consumer-facing options? Do we need backwards compatibility? @pastelsky
|
@pastelsky Thanks, I have rebased. I left a few comments with some questions before we should proceed though. How can we test this well? |
|
@AndersDJohnson Thanks! Note that we're in the middle of migrating our codebase to typescript, so I'm going to hold off this change for a while to prevent conflicts. We don't have good tests for Bundlephobia (the UI / server) today, unfortunately — would love it if someone initiated a setup. Typescript should give us additional safety once that's merged. |
Not sure yet if I applied these changes correctly, or if they will work properly, so would likely need help testing & reviewing.
But just thought I'd help start the process of considering a potential rename to more inclusive terms like blocklist and allowlist, as several other projects and teams are doing lately. If it's not feasible to take this on now, or there is disagreement, then these proposed changes can be dismissed.
For reference: