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

Repo Proposal: creating a metal-jsx-extras repository #239

Closed
bryceosterhaus opened this issue May 19, 2017 · 5 comments
Closed

Repo Proposal: creating a metal-jsx-extras repository #239

bryceosterhaus opened this issue May 19, 2017 · 5 comments

Comments

@bryceosterhaus
Copy link
Member

I was looking into #205 and #127 and trying to figure out the best way to solve the two. After some research and talking with @mthadley, we thought it might be best to create a separate metal-jsx-extras repo that would contain extra(non-core) functionality for metal-jsx. This would be a place we could export a util function like otherProps(#205) and util components like <DangerouslySetHTML />(#127) since these are would be commonly used in along side of metal-jsx.

Usage:

import Component from 'metal-jsx';
import {DangerouslySetHTML, otherProps} from 'metal-jsx';

class MyComponent extends Component {
  render() {
    const html = '<h1>Hello World!</h1>';

    return (
      <DangerouslySetHTML  {...otherProps(this)} html={html} />
    );
  }
}

export default MyComponent;

@eduardolundgren, @jbalsas what are your thoughts on this? If it sounds like a good idea to you, I can go ahead and get it going.

@jbalsas
Copy link
Contributor

jbalsas commented May 23, 2017

Hey @bryceosterhaus, after reading through #127 and #205, this comment by @mthadley particularly resonates:

My first thought was that this could just be a separate module, completely outside of metal-jsx. However because it was such a strong pattern for us, I thought it might be good to propose it here first.

I guess the same might be said about the DangerouslySetHTML proposal, so my preference would be to have those inside the current metal-jsx repo. Inside an extras folder if you wish, but within the same package. We can always extract it later if needed.

What do you think?

@bryceosterhaus
Copy link
Member Author

I would be fine with putting it in the metal-jsx repo. However, the reason I would be okay with it is because I use both of those extras. For someone who may not use either, I could imagine that they wouldn't want those included with metal-jsx.

So if we think "everyone will use these extras in all of their projects" then I would say put it in the jsx repo. But if we anticipate that only 50% will use these extras for their project, then I would say that we should put it in a different place.

@jbalsas
Copy link
Contributor

jbalsas commented May 23, 2017

I think you guys have leveraged metal-jsx more than anyone else, so it's just fair that you drive some of the guidelines. If that makes sense for you, it probably makes sense for more than just the 50% of the people.

Also, depending on the build process, you probably will be getting rid of that dead weight anyways...

@mthadley
Copy link

I think our initial guess was that most would have not wanted to see these kinds of things added to the metal-jsx repo, but I think it's reasonable either way. These are very tiny additions anyways.

As far as them being framework bloat, I think the build process would need to have some kind of tree-shaking to remove them if they are unused. But again, these are very small additions to the library.

@jbalsas
Copy link
Contributor

jbalsas commented Jul 7, 2017

Hey, I think we did merge this directly in metal-jsx, didn't we? Closing here 👍

@jbalsas jbalsas closed this as completed Jul 7, 2017
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

No branches or pull requests

3 participants