Default values being overwritten by an undefined value. [metal-jsx]

[bryceosterhaus]

When using default values for props, I would expect that if undefined is passed as a prop from the parent component, it would be ignored and the component would use its default prop value. However, this is not the case and currently it uses the undefined value.

Here is an example seen in metal, Metal Example

I think the react functionality for default props is what I would expect. React Example.

[kienD]

I think that this is a good idea. I've run into this issue before.

[bryceosterhaus]

@kienD do you use a certain workaround for this?

[kienD]

@bryceosterhaus,

As of now, I'm just setting defaults value in the parent component so that the child component won't receive an undefined value.

[robframpton]

Copying from #260

I did some digging, and this behavior is an unintended side effect of f1e8847. That fix intended to ensure that validators are run for undefined values, in case isRequired was being used. However, it also makes it so that undefined will overwrite default values.

I ran @bryceosterhaus' tests against the commit right before f1e8847 and it passes.

Since this hasn't always been the behavior, I think we should go ahead and fix it. We can still run it against some larger code bases though, just to be safe.

[jbalsas]

Merged into develop and scheduled for 2.14.0!

[bryceosterhaus]

@jbalsas should we be closing issues that are not released yet? Technically they are not "fixed" yet since it is not released. Only bring this up because I just spent 20 min debugging my code thinking this was fixed because it was closed 🤓

[jbalsas]

Hey @bryceosterhaus! Wasn't my message clear enough?

Merged into develop and scheduled for 2.14.0!

We're trying to adopt a git-flow-like branching strategy, but that makes working with issues that are merged into the develop branch a bit complicated, because they stay open until merged to master... do you know how this gets handled in other projects? Any suggestion as to how to better communicate or organize this?

@Robert-Frampton, @brunobasto, thoughts?

[bryceosterhaus]

I'm not super familiar with git-flow, so I can't really speak to that.

I'm not totally sure what other repos do or formal processes to follow for github issues. From my experience though, as a user of a project, I expect an issue to be closed/resolved once the issue is fixed and released for use.

What I think of when I see issues:
closed: fixed and released
open: bug that exists in the latest release.

I could definitely be wrong on this, I just was surprised to see this as closed when the issue is technically not fixed for use.

[jbalsas]

This is exactly how we operate on Liferay Portal, for one... we close issues when they get merged to the master branch, which does not mean at all they have gotten released.

That's how it would work also when tagging commits with Fixes #123 kind of messages, that it will auto-close when merged on master, before release. That's how we used to operate around here before...

I see what you mean, and I don't want you to spend more time debugging in search for unreleased features, so I'd rather look at some way of better tagging things... maybe keep them open but with a staged-for-release label or something.

In any case, we'd need to do some research on other projects and good practices.

Thanks for bringing this up!

[bryceosterhaus]

I looked around at a few other projects, couldn't find anything that was formalized though. More labels is a good idea though, that was one thing that was pretty consistent in the few repos I looked at.

[yuchi]

My usual 2¢ follow.

Usually an issue is closed as soon as a solution hits the repository.

Usually projects have a very fast release cycles but tend to aggregate more issues in a single release. If the issue has a workaround is usually enough to comunicate the planned release of the fix in a specific version and ask the submitter to wait for it.

If a fix, once release, doesn’t actually solves the issue it originated from, then the submitter has the right to re-open the issue to bring more context in.

[brunobasto]

I like the idea of using labels.

Another idea is using milestones. Milestones can give a good idea to whoever opened the issue in what version they can expect that issue to be solved.

Not sure how that would work for us, though.

[jbalsas]

@brunobasto, we are indeed starting to use milestones. We are almost done with 2.14.0. The problem is that to see progress in a milestone, tickets need to be closed. That's what I was doing, to make sure everything we plan was accounted for, but as @bryceosterhaus points out, this may cause some confusion, so we probably need to do something else on top :)

@yuchi we are trying to address the release cycle on metal.js projects. We are not only looking at faster releases, but also more stable and predictable. We don't want to swamp npm with tiny releases for every fixed bug. IMHO, this depreciates the value of the library releases as they often contain just a minor fix. If someone wants to always stay current, a dependency on package#develop can be used 😉

We are crafting 2.14.0 and working on documentation as well to serve as the foundation of this new cycle :)

[bryceosterhaus]

Saw this label on a repo, thought it was helpful. https://github.com/sequelpro/sequelpro/labels/Fixed-AwaitingRelease

[brunobasto]

@brunobasto, we are indeed starting to use milestones. We are almost done with 2.14.0. The problem is that to see progress in a milestone, tickets need to be closed. That's what I was doing, to make sure everything we plan was accounted for, but as @bryceosterhaus points out, this may cause some confusion, so we probably need to do something else on top :)

I see. Even if we go with the label and leave the issue open, we would never see any progress during the lifecycle of the milestone, right? Then the milestone would simply represent the version the issue will be released. But we won't see the progress information.

[zenorocha]

What if we just release a new version every time a new pull request is merged?

Anyways, if we go on this direction of a develop branch, then we need to make sure that all repos have that branch configured to be the default one. Otherwise, new contributors won't know that they were supposed to send the PR over there. Plus, it might solve this problem of issues staying open until merged to master that @jbalsas mentioned.

[jbalsas]

I personally don't like the release-for-every-change approach. Maybe it's just me, but I don't trust releases done that way. The cost of updating dependencies in projects is not something to ignore. We don't even keep the pace ourselves with our own releases, so I think it's best if we can ensure we release:

• With important and noticeable chunks of features and fixes that will be worth your time
• In a minimum given timeline (at least once a month no matter how many fixes went in)