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

Convert to object-first convention for use with fast pipe #77

Merged
merged 14 commits into from Aug 16, 2018
Merged

Conversation

zploskey
Copy link
Owner

@zploskey zploskey commented Aug 15, 2018

Convert all bs.send.pipe externals to bs.send for use with fast pipe (-> or |.). For bindings with multiple arguments the object is moved from the last to the first argument.

This is a breaking change, but one that I think is worthwhile. It eliminates a extra JS function calls in the generated JavaScript. Switching |> to -> is straight forward and the compiler will tell exactly where it needs to be changed.

In a few instances it was necessary to add a final unit argument after one or more optional arguments. Again, the compiler will complain about this and it's a straightforward fix.

Fixes #36.

These ignored jest assertions were never being run.
Do a bit of refactoring while we're here.
return (function (eta) {
return BrowserFetcher$BsPuppeteer.download("533271", undefined, eta);
})(browserFetcher[0]).then((function (info) {
return browserFetcher[0].download("533271", undefined).then((function (info) {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't try to tell me that's not an improvement in the compiled output! Now if we could just do something about that indentation.

@zploskey zploskey changed the title [WIP] Convert to object-first convention for use with fast pipe Convert to object-first convention for use with fast pipe Aug 16, 2018
@zploskey zploskey merged commit 5f63b34 into master Aug 16, 2018
@zploskey zploskey deleted the fastpipe branch August 16, 2018 16:11
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

Successfully merging this pull request may close these issues.

Remove use of bs.send.pipe
1 participant