feat(takeWhile): add `inclusive` option to the operator which causes it to emit the final value #4115

idosela · 2018-09-07T18:45:31Z

Description:
By default, the value that causes the predicate to return false is not emitted.

This change adds an inclusive option which changes the behavior of the operator to include the final value which made the predicate return false.
Typical use case: Polling an API until the response contains a certain value, and then doing something with the response.

Related issue: #4000

ci-reporter · 2018-09-10T14:38:20Z

The build is failing

✨ Good work on this PR so far! ✨ Unfortunately, the Travis CI build is failing as of cc38746. Here's the output:

`npm test`

> @reactivex/rxjs@6.3.2 test /home/travis/build/ReactiveX/rxjs
> cross-env TS_NODE_PROJECT=spec/tsconfig.json mocha --opts spec/support/default.opts "spec/**/*-spec.ts"


/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:250
    return new TSError(diagnosticText, diagnosticCodes)
           ^
TSError: ⨯ Unable to compile TypeScript:
spec/operators/takeWhile-spec.ts(81,30): error TS2554: Expected 1 arguments, but got 2.

    at createTSError (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:250:12)
    at getOutput (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:358:40)
    at Object.compile (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:545:11)
    at Module.m._compile (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:430:43)
    at Module._extensions..js (module.js:664:10)
    at Object.require.extensions.(anonymous function) [as .ts] (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:433:12)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Module.require (module.js:597:17)
    at require (internal/module.js:11:18)
    at /home/travis/build/ReactiveX/rxjs/node_modules/mocha/lib/mocha.js:231:27
    at Array.forEach (<anonymous>)
    at Mocha.loadFiles (/home/travis/build/ReactiveX/rxjs/node_modules/mocha/lib/mocha.js:228:14)
    at Mocha.run (/home/travis/build/ReactiveX/rxjs/node_modules/mocha/lib/mocha.js:536:10)
    at Object.<anonymous> (/home/travis/build/ReactiveX/rxjs/node_modules/mocha/bin/_mocha:582:18)
    at Module._compile (module.js:653:30)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Function.Module.runMain (module.js:694:10)
    at startup (bootstrap_node.js:204:16)
    at bootstrap_node.js:625:3

I'm sure you can fix it! If you need help, don't hesitate to ask a maintainer of the project!

Failed build for 0a9291a

`npm test`

> @reactivex/rxjs@6.3.2 test /home/travis/build/ReactiveX/rxjs
> cross-env TS_NODE_PROJECT=spec/tsconfig.json mocha --opts spec/support/default.opts "spec/**/*-spec.ts"


/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:250
    return new TSError(diagnosticText, diagnosticCodes)
           ^
TSError: ⨯ Unable to compile TypeScript:
spec/operators/takeWhile-spec.ts(81,30): error TS2554: Expected 1 arguments, but got 2.

    at createTSError (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:250:12)
    at getOutput (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:358:40)
    at Object.compile (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:545:11)
    at Module.m._compile (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:430:43)
    at Module._extensions..js (module.js:663:10)
    at Object.require.extensions.(anonymous function) [as .ts] (/home/travis/build/ReactiveX/rxjs/node_modules/ts-node/src/index.ts:433:12)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Module.require (module.js:596:17)
    at require (internal/module.js:11:18)
    at /home/travis/build/ReactiveX/rxjs/node_modules/mocha/lib/mocha.js:231:27
    at Array.forEach (<anonymous>)
    at Mocha.loadFiles (/home/travis/build/ReactiveX/rxjs/node_modules/mocha/lib/mocha.js:228:14)
    at Mocha.run (/home/travis/build/ReactiveX/rxjs/node_modules/mocha/lib/mocha.js:536:10)
    at Object.<anonymous> (/home/travis/build/ReactiveX/rxjs/node_modules/mocha/bin/_mocha:582:18)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Function.Module.runMain (module.js:693:10)
    at startup (bootstrap_node.js:191:16)
    at bootstrap_node.js:612:3

This comment was automagically generated by ci-reporter. If you see a problem, open an issue here.

idosela · 2018-09-10T20:38:33Z

@benlesh I keep getting this error:
https://travis-ci.org/ReactiveX/rxjs/jobs/426812416

As far as I can tell I updated all of the required files, and all test are passing when I run them on my machine. I'm probably missing something super-obvious, but I can't figure it out. Any help would be appreciated.

compat/operator/takeWhile.ts

timdeschryver · 2018-09-20T06:09:51Z

I think these changes should also be added to the dtslint test specs, @cartant ?

cartant · 2018-09-20T06:22:41Z

@timdeschryver Yes, it will need dtslint tests. However, ATM, I'm more concerned with the changes merged in #4085 seeming to have been undone. The signature that accepts user-defined type guard appears to have been lost.

src/internal/operators/takeWhile.ts

coveralls · 2018-09-20T14:52:20Z

Coverage increased (+0.003%) to 96.97% when pulling f7b3d7d on idosela:take-while-inclusive into 6a2b524 on ReactiveX:master.

idosela · 2018-09-26T21:43:28Z

@cartant I've made the required fixes. Can you please take a look and let me know if there are any other issues I need to address?

src/internal/operators/takeWhile.ts

…es to emit final value By default, the value that causes the predicate to return `false` is not emitted. This change adds an `inclusive` option which changes the behavior of the operator to include the final value which made the predicate return `false`. Typical use case: Polling an API until the response contains a certain value, and then doing something with the response.

idosela · 2018-10-10T02:44:02Z

@cartant Can you please take another look and let me know if there is anything else I need to fix?

cartant · 2018-10-10T03:29:57Z

@idosela Sure. I'll look at it again, later today. Sorry for forgetting about it.

cartant

LGTM

idosela · 2018-11-05T19:22:34Z

@benlesh @cartant The PR has been approved for a while. When can I expect it to be merged into master?

felixfbecker · 2018-12-14T00:40:47Z

In need of this too :)

benlesh · 2019-01-30T03:21:34Z

Thanks, @idosela!

idosela changed the title ~~feat(takeWhile): add an inclusive option to the operator which causes to emit final value~~ feat(takeWhile): add inclusive option to the operator which causes it to emit the final value Sep 7, 2018

benlesh requested changes Sep 12, 2018

View reviewed changes

compat/operator/takeWhile.ts Outdated Show resolved Hide resolved

cartant requested changes Sep 20, 2018

View reviewed changes

src/internal/operators/takeWhile.ts Show resolved Hide resolved

benlesh approved these changes Sep 25, 2018

View reviewed changes

benlesh added the Next Minor Version label Sep 25, 2018

cartant requested changes Sep 27, 2018

View reviewed changes

src/internal/operators/takeWhile.ts Outdated Show resolved Hide resolved

cartant mentioned this pull request Sep 27, 2018

Request for takeUntil with predicate function #2420

Closed

idosela force-pushed the take-while-inclusive branch from 6583875 to c183085 Compare September 27, 2018 19:29

idosela force-pushed the take-while-inclusive branch from c183085 to f7b3d7d Compare September 27, 2018 19:32

cartant approved these changes Oct 10, 2018

View reviewed changes

benlesh merged commit 6e7f407 into ReactiveX:master Jan 30, 2019

cartant mentioned this pull request Feb 18, 2019

improve takeUntil and takeWhile to have overload to includeLastValue = true #4000

Closed

lock bot locked as resolved and limited conversation to collaborators Mar 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(takeWhile): add `inclusive` option to the operator which causes it to emit the final value #4115

feat(takeWhile): add `inclusive` option to the operator which causes it to emit the final value #4115

idosela commented Sep 7, 2018

ci-reporter bot commented Sep 10, 2018 •

edited

`npm test`

idosela commented Sep 10, 2018

timdeschryver commented Sep 20, 2018

cartant commented Sep 20, 2018

coveralls commented Sep 20, 2018 •

edited

idosela commented Sep 26, 2018

idosela commented Oct 10, 2018

cartant commented Oct 10, 2018

cartant left a comment

idosela commented Nov 5, 2018

felixfbecker commented Dec 14, 2018

benlesh commented Jan 30, 2019

feat(takeWhile): add inclusive option to the operator which causes it to emit the final value #4115

feat(takeWhile): add inclusive option to the operator which causes it to emit the final value #4115

Conversation

idosela commented Sep 7, 2018

ci-reporter bot commented Sep 10, 2018 • edited

The build is failing

npm test

npm test

This comment was automagically generated by ci-reporter. If you see a problem, open an issue here.

idosela commented Sep 10, 2018

timdeschryver commented Sep 20, 2018

cartant commented Sep 20, 2018

coveralls commented Sep 20, 2018 • edited

idosela commented Sep 26, 2018

idosela commented Oct 10, 2018

cartant commented Oct 10, 2018

cartant left a comment

Choose a reason for hiding this comment

idosela commented Nov 5, 2018

felixfbecker commented Dec 14, 2018

benlesh commented Jan 30, 2019

feat(takeWhile): add `inclusive` option to the operator which causes it to emit the final value #4115

feat(takeWhile): add `inclusive` option to the operator which causes it to emit the final value #4115

ci-reporter bot commented Sep 10, 2018 •

edited

`npm test`

`npm test`

coveralls commented Sep 20, 2018 •

edited