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 a core utility to strip magic keys and other keys from a component #2472

Closed
dead-claudia opened this issue Jul 12, 2019 · 4 comments · Fixed by #2538
Closed

Add a core utility to strip magic keys and other keys from a component #2472

dead-claudia opened this issue Jul 12, 2019 · 4 comments · Fixed by #2538
Assignees
Labels
Type: Enhancement For any feature request or suggestion that isn't a bug fix
Milestone

Comments

@dead-claudia
Copy link
Member

Mithril version:

Browser and OS:

Project:

Is this something you're interested in implementing yourself? Yes

Description

Continuation of #1986

Specifically, I'm suggesting bringing in this utility into core. Already just got bit writing a built-in utility, and I know not everyone reads the docs closely. Plus, having a core utility that contradicts this doesn't really make sense, and I don't expect everyone to read the docs closely enough to pick this up. So instead, we should just provide a built-in utility to strip all that crap out for people.

Why

Well, if you want to know how easy it is to get bit without realizing it, take a look at #2471.

Possible Implementation

https://github.com/isiahmeadows/mithril-helpers/blob/master/censor.js with the alterations needed to fuse into core. (It's 324 bytes min+gzip standalone.)

Open Questions

@dead-claudia dead-claudia added the Type: Enhancement For any feature request or suggestion that isn't a bug fix label Jul 12, 2019
@dead-claudia dead-claudia self-assigned this Jul 12, 2019
@project-bot project-bot bot added this to Needs triage in Triage/bugs Jul 12, 2019
@dead-claudia
Copy link
Member Author

And to append to that, the docs don't make it clear it applies even in cases like this, the moral equivalent of what I did:

const {component, options, href, ...rest} = vnode.attrs

return m(component, {
    ...rest,
    href: prefix + href,
    onclick() { ... }
}, vnode.children)

@dead-claudia
Copy link
Member Author

BTW, I don't plan to "fix" m.route.Link to follow this, but instead plan to add a utility to get out of the way of users. It won't likely hit v2.0.0, but my hope is that it hits v2.1.0.

/cc @barneycarroll @StephanHoyer if you all have thoughts.

@dead-claudia
Copy link
Member Author

Also, I don't plan to implement all of this, just implementing m.censor. It's relatively small, somewhat surprisingly so for being as optimized as it is.

@dead-claudia
Copy link
Member Author

I just did some measurements, and it'd increase the min+gzip bundle size from 9546 bytes to 9673 bytes, an increase of just under 130 bytes. So it's relatively small.

@dead-claudia dead-claudia added this to Under consideration in Feature requests/Suggestions via automation Jul 24, 2019
@dead-claudia dead-claudia removed this from Needs triage in Triage/bugs Jul 24, 2019
@dead-claudia dead-claudia added this to the 2.1.0 milestone Jul 24, 2019
Feature requests/Suggestions automation moved this from Under consideration to Completed/Declined Sep 30, 2019
@orbitbot orbitbot added this to Done in 2.1.0 release Jul 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement For any feature request or suggestion that isn't a bug fix
Projects
2.1.0 release
  
Done
Development

Successfully merging a pull request may close this issue.

1 participant