Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fix #595, #427, #602, refactor ZoneAwareError property copy logic #597

Merged
merged 1 commit into from
Jan 17, 2017

Conversation

JiaLiPassion
Copy link
Collaborator

@JiaLiPassion JiaLiPassion commented Jan 13, 2017

fix #595,#427.
In previous PR to fix #546, we copy every property from NativeError to this (ZoneAwareError) with following logic.

this[key] = error[key]

and such logic will have conflict with following angular2 BaseError logic.

  class BaseError extends Error {
      /** @internal **/
      _nativeError: Error;

      constructor(message: string) {
        super(message);
        const nativeError = new Error(message) as any as Error;
        this._nativeError = nativeError;
      }

      get message() {
        return this._nativeError.message;
      }
      set message(message) {
        this._nativeError.message = message;
      }
      get name() {
        return this._nativeError.name;
      }
      get stack() {
        return (this._nativeError as any).stack;
      }
      set stack(value) {
        (this._nativeError as any).stack = value;
      }
      toString() {
        return this._nativeError.toString();
      }
}

BaseError has such hack logic to make sure this is used.
because BaseError's constructor will call super , and super will call copy, copy will then call
BaseError's setter, but this._nativeError is still undefined, so it will cause Exception.

In this PR, instead of copy property values with assignAll, I create propertyDescriptor to delegate to native error.

Object.defineProperty(this, key, {
    configurable: true, enumerable: true,
    get, set
})

And because ZoneAwareError already handle Error this constructor issue, maybe in the future, angular2 BaseError don't need to handle such issue again.
angular/angular#13908

@JiaLiPassion
Copy link
Collaborator Author

JiaLiPassion commented Jan 13, 2017

@mhevery, sorry for breaking changes again, I'll run test on angular check whether the fix is ok.
Please review.

With this PR #597, and PR #594, angular test and angular-cli generated projects test has passed.

@JiaLiPassion JiaLiPassion force-pushed the issue-595 branch 4 times, most recently from 1f3cdb5 to 3b05a99 Compare January 14, 2017 07:35
@JiaLiPassion JiaLiPassion changed the title fix #595, refactor ZoneAwareError property copy logic fix #595, #427, #602, refactor ZoneAwareError property copy logic Jan 16, 2017
@cocaux
Copy link

cocaux commented Jan 16, 2017

Hi guys,
When will this be released?
Thanks
Coco

@JiaLiPassion JiaLiPassion force-pushed the issue-595 branch 2 times, most recently from d5d4f67 to 5c3362d Compare January 16, 2017 23:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants