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

Breaking change in v1.1.0 #782

Closed
ljharb opened this issue Jun 7, 2015 · 7 comments
Closed

Breaking change in v1.1.0 #782

ljharb opened this issue Jun 7, 2015 · 7 comments

Comments

@ljharb
Copy link

ljharb commented Jun 7, 2015

caolan/forms’ tests pass on v1.0.0, but break on v1.1.0 and v1.2.0.

I'm still digging for the bug, but this means that v1.1.0 should instead have been v2.0.0 - once we find the fix, both the 1.1 and 1.2 branches will need a patch release to revert the breakage.

@ljharb
Copy link
Author

ljharb commented Jun 7, 2015

Using git bisect, bf71c8c appears to be the commit that broke it - ping @aearly

ljharb added a commit to caolan/forms that referenced this issue Jun 7, 2015
@aearly
Copy link
Collaborator

aearly commented Jun 7, 2015

Do you have an isolated test case for what is breaking? I'm apprehensive there's a widely used corner case in async that async's tests don't cover...

@ljharb
Copy link
Author

ljharb commented Jun 7, 2015

If you clone the forms repo, and run npm install && npm install async@1.1.0 && node test/test-render.js you'll see the 4 tests that fail.

The gist seems to be "expected 4 assertions, got 0" which is implying that a previous async.forEach/forEachSeries call would iterate 4 times whereas now it doesn't iterate.

@aearly
Copy link
Collaborator

aearly commented Jun 8, 2015

The issue is that the forms tests are relying on async.eachSeries calling its callback on the same tick if there is a single synchronous validator in field.bind. This wasn't a guarantee async was making, but since people are relying on it, I'll make a fix.

@ljharb
Copy link
Author

ljharb commented Jun 8, 2015

OK - I'll certainly correct that, but "same tick" and "next tick" changes are definitely a breaking change.

@aearly aearly closed this as completed in 0450c3b Jun 8, 2015
aearly pushed a commit that referenced this issue Jun 8, 2015
@ljharb
Copy link
Author

ljharb commented Jun 8, 2015

Thanks!

@aearly
Copy link
Collaborator

aearly commented Jun 8, 2015

Sure thing. v1.1.1 and v1.2.1 published

ljharb added a commit to caolan/forms that referenced this issue Jun 8, 2015
(since caolan/async#782 is resolved)
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

2 participants