Update TS: Alternative Error Fix #2614

benlesh · 2017-05-25T00:13:14Z

This is the same as #2606, but updates the custom error classes to use setPrototypeOf

Pros:

instanceof still works
fewer breaking changes
call stacks and other goodness preserved for Chrome DevTools

Cons:

IE10 users will need an Object.setPrototypeOf polyfill.
setPrototypeOf creates a "slow object" in V8... deoptimized as a hashtable, to my understanding.

Other things:

The Object.setPrototypeOf polyfill is trivial:

Object.setPrototypeOf || Object.setPrototypeOf = function(a, b) { a.__proto__ = b; return a; }

cc @trxcllnt

- adds `tslib` import to `Rx.ts` - adds `--noEmitHelpers` to build output closes #2605

BREAKING CHANGE: The following types no longer exist: `EmptyError`, `UnsubscriptionError`, `ArgumentOutOfRangeError`, `ObjectUnsubcribedError`, `TimeoutError`, `AjaxError`, `AjaxTimeoutError`. There are now utilities for creating and testing under `Rx.Util` for example: `Rx.Util.createTimeoutError()` and `Rx.Util.isTimeoutError(err)` etc. closes #2612 related #2582

This reverts commit 13767c3.

BREAKING CHANGE: IE10 and lower will need to polyfill `Object.setPrototypeOf`

rxjs-bot · 2017-05-25T00:16:52Z

	Warnings
⚠️	commit message does not follows conventional change log (1)

(1) : RxJS uses conventional change log to generate changelog automatically. It seems some of commit messages are not following those, please check contributing guideline and update commit messages.

Generated by 🚫 dangerJS

coveralls · 2017-05-25T00:21:19Z

Coverage decreased (-0.003%) to 97.732% when pulling d242156 on benlesh:errorAlternative into da96b6a on ReactiveX:master.

- updates `responseType` in ajax observable area to be `XMLHttpRequestResponseType` BREAKING CHANGE: TypeScript 2.3 now required BREAKING CHANGE: `AjaxRequest`'s `responseType` property is now `XMLHttpRequestResponseType` rather than `string` to support TS 2.3

TypeScript now distributes typings via npm.

Fix buffer operator not emitting the last buffer when the source completes. This is closer to rxjs v4* behaviour and matches the behaviour of the other buffer operators. The _complete method is very similar to thoses in bufferCount, bufferTime and bufferWhen. *rxjs v4 will always emit the buffer if the source completes even if the buffer is empty. This fix only emits if the buffer is non empty. BREAKING CHANGE: The `buffer()` operator now emits what's partially buffered when the source completes. This is closer to rxjs v4* behaviour and matches the v5 behaviour of the other buffer operators.

Brooooooklyn · 2017-05-26T07:41:30Z

src/observable/dom/AjaxObservable.ts

@@ -5,6 +5,7 @@ import { Observable } from '../../Observable';
 import { Subscriber } from '../../Subscriber';
 import { TeardownLogic } from '../../Subscription';
 import { MapOperator } from '../../operator/map';
+import 'tslib';


don't need this

Brooooooklyn · 2017-05-26T07:41:53Z

src/Rx.ts

@@ -1,4 +1,6 @@
 /* tslint:disable:no-unused-variable */
+import 'tslib';


don't need this either

Widdershin · 2017-05-27T00:31:12Z

The failure from markdown-doctest appears to be a module issue.

It's failing when trying to import Rx from 'rxjs/Rx'.

This is the example from the README that's failing:

import Rx from 'rxjs/Rx';

Rx.Observable.of(1,2,3)

After being compiled by babel this is how it looks:

var _Rx = require('rxjs/Rx');

var _Rx2 = _interopRequireDefault(_Rx);

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

_Rx2.default.Observable.of(1, 2, 3);

This started breaking with TypeScript 2.3 because now require('rxjs/Rx').__esModule is true so _interopRequireDefault goes down a different branch in the ternary.

This line now appears at the top of the compiled output:

Object.defineProperty(exports, "__esModule", { value: true });

Arguably this is reasonable behaviour, as import Rx from 'rxjs/rx'; is importing a default, and the module in question has no exported default.

I'm unsure why __esModule is now being set to true, seems like this must have changed in TypeScript 2.3 or is configured somewhere I can't see.

One possible fix is to change import Rx from 'rxjs/Rx' to import * as Rx from 'rxjs/Rx'.

Another would be to export a default, but that seems like it would be a lot of duplication.

I think this failure is indicative of a problem that could impact real users. I know TypeScript is strict about importing using that syntax from modules with no default. I think in the past babel allows you to use that syntax for commonjs modules, but maybe it's always been based on __esModule.

…Alternative

benlesh · 2017-06-14T17:38:27Z

Thanks @Widdershin ... I think we can actually just make that change. It's only in two spots I believe.

- had to had new tslint rules specific for tests to disable no-unused-expression because it didn't like the chai assertions such as `expect(foo).to.be.true`, as it wanted `true` to be set or called as a function

coveralls · 2017-06-14T20:33:29Z

Coverage decreased (-0.003%) to 97.734% when pulling 5151d80 on benlesh:errorAlternative into 9bc0348 on ReactiveX:next.

coveralls · 2017-06-14T20:55:10Z

Coverage decreased (-0.003%) to 97.734% when pulling 5151d80 on benlesh:errorAlternative into 9bc0348 on ReactiveX:next.

Error.stack can potentially trigger expensive prepareStackTrace calls, Observable.timeout would crate a TimeoutError on use, before the actual error occurs. This eases tracing the location of the timeout, but with the current implementation, slows down the application when using many .timeout operators throughout the code. Manual cherry-pick of ReactiveX#2614 commit d242156

lock · 2018-06-06T09:40:47Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

benlesh added 7 commits May 24, 2017 13:17

refactor(Rx): stop duplicating TS class helpers in each CJS file

c990b4a

- adds `tslib` import to `Rx.ts` - adds `--noEmitHelpers` to build output closes #2605

chore(tslib): update tslib to 1.7.*

33b0ffc

Merge branch 'master' into noEmitHelpers

094c8e0

Revert "feat(errors): all custom errors are now just Error objects"

2f89f7c

This reverts commit 13767c3.

refactor(assign-spec): fix type checking error for Object.assign

42928e3

feat(errors): refactor errors to use Object.setPrototypeOf

d242156

BREAKING CHANGE: IE10 and lower will need to polyfill `Object.setPrototypeOf`

benlesh and others added 3 commits May 25, 2017 09:53

chore(typescript): Cleanup usage of typings package.

3b31b29

TypeScript now distributes typings via npm.

chore(typings): update lodash typings for TS 2.3

0c90bca

This was referenced May 25, 2017

chore(typescript): Cleanup usage of typings package. #2610

Closed

Update to TypeScript 2.1, Update Errors, Breaking Changes! #2606

Closed

benlesh changed the base branch from master to next May 25, 2017 17:16

benlesh and others added 2 commits May 25, 2017 10:18

Merge branch 'next' into errorAlternative

2a25ad6

Brooooooklyn reviewed May 26, 2017

View reviewed changes

src/Rx.ts

@@ -1,4 +1,6 @@

/* tslint:disable:no-unused-variable */

import 'tslib';

Copy link

Contributor

Brooooooklyn May 26, 2017

Choose a reason for hiding this comment

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

don't need this either

kwonoj mentioned this pull request May 29, 2017

Fix TS definition of publish & multicast #2616

Closed

benlesh added 2 commits June 14, 2017 10:33

chore(docs): fix issue causing errors with doc test runner

cae9b6a

Merge branch 'errorAlternative' of github.com:benlesh/rxjs into error…

9294eda

…Alternative

chore(tslint): update tslint

5151d80

- had to had new tslint rules specific for tests to disable no-unused-expression because it didn't like the chai assertions such as `expect(foo).to.be.true`, as it wanted `true` to be set or called as a function

chore(merge): merging next into feature branch

8caab8f

benlesh merged commit 4515d30 into ReactiveX:next Jun 14, 2017

benlesh deleted the errorAlternative branch June 14, 2017 21:20

hermanbanken mentioned this pull request Aug 7, 2017

feat(error): Lazy Error.stack usage #2781

Closed

lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update TS: Alternative Error Fix #2614

Update TS: Alternative Error Fix #2614

benlesh commented May 25, 2017

rxjs-bot commented May 25, 2017 •

edited

Loading

coveralls commented May 25, 2017

Brooooooklyn May 26, 2017

Brooooooklyn May 26, 2017

Widdershin commented May 27, 2017

benlesh commented Jun 14, 2017

coveralls commented Jun 14, 2017

coveralls commented Jun 14, 2017 •

edited

Loading

lock bot commented Jun 6, 2018

		@@ -1,4 +1,6 @@
		/* tslint:disable:no-unused-variable */
		import 'tslib';

Update TS: Alternative Error Fix #2614

Update TS: Alternative Error Fix #2614

Conversation

benlesh commented May 25, 2017

Pros:

Cons:

Other things:

rxjs-bot commented May 25, 2017 • edited Loading

coveralls commented May 25, 2017

Brooooooklyn May 26, 2017

Choose a reason for hiding this comment

Brooooooklyn May 26, 2017

Choose a reason for hiding this comment

Widdershin commented May 27, 2017

benlesh commented Jun 14, 2017

coveralls commented Jun 14, 2017

coveralls commented Jun 14, 2017 • edited Loading

lock bot commented Jun 6, 2018

rxjs-bot commented May 25, 2017 •

edited

Loading

coveralls commented Jun 14, 2017 •

edited

Loading