ClojureScript

Upgrade ExceptionInfo to proper deftype

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

Currently ExceptionInfo is implemented as a raw constructor function which inherits from js/Error with some ad-hoc javascript-level patches to satisfy a tiny subset of deftype functionality (mainly for printing).

Unfortunately this does not play well with cljs-devtools[1]. This problem surfaced when I started playing with ExceptionInfo and cljs-devtools v0.8 which newly supports printing deftypes[2]. ExceptionInfo does not contain getBasis, cljs$lang$type, cljs$lang$ctorStr and similar machinery.

My proposed patch implements ExceptionInfo as a proper deftype and does some patch-work to provide backward compatibility. I'm pretty sure we must not break current contract of ExceptionInfo constructor accepting 3 args and synthesizing other fields on the fly in constructor.

Implementation details:
1) first we define ExceptionInfo as normal deftype (to get a template)
2) then we remember reference to ExceptionInfo in ExceptionInfoTypeTemplate
3) then we redefine ExceptionInfo with raw constructor function which should mimic original behaviour (by scraping newly created js/Error instance, but calling ExceptionInfoTypeTemplate to do proper deftype initialization)
4) then we copy keys from ExceptionInfoTypeTemplate over ExceptionInfo
5) then we set ExceptionInfo's prototype to be ExceptionInfoTypeTemplate's prototype
6) then we fix ExceptionInfo's prototype's constructor to point to our re-defined constructor function
7) then we patch ExceptionInfo's prototype to inherit from js/Error (note this clobbers ExceptionInfoTypeTemplate as well - but we don't care about it)

This effectively gives us properly working ExceptionInfo deftype with redefined constructor function wrapping deftype's constructor for backwards compatibility.
We also patch ExceptionInfo's prototype to inherit from js/Error the same was as original code did.

Note: With working deftype, we can move IPrintWithWriter and toString implementation to the deftype itself.

[1] https://github.com/binaryage/cljs-devtools/issues/23
[2] https://github.com/binaryage/cljs-devtools/releases/tag/v0.8.0

Activity

Antonin Hildebrand made changes -
Field Original Value New Value
Summary Upgrade ExceptionInfo into proper deftype Upgrade ExceptionInfo to proper deftype
Antonin Hildebrand made changes -
Attachment CLJS-1722.patch [ 15807 ]
Description Currently ExceptionInfo is implemented as a raw constructor function which inherits from js/Error with some ad-hoc javascript-level patches to satisfy a tiny subset of deftype functionality (mainly for printing).

Unfortunately this does not play well with cljs-devtools[1]. This problem surfaced when I started playing with ExceptionInfo and cljs-devtools v0.8 which newly supports printing deftypes[2]. ExceptionInfo does not contain getBasis, cljs$lang$type, cljs$lang$ctorStr and similar machinery.

My proposed patch implements ExceptionInfo as a proper deftype and does some patch-work to provide backward compatibility. I'm pretty sure we must not break current contract of ExceptionInfo constructor accepting 3 args and synthesizing other fields on the fly in constructor.

Implementation details:
1) first we define ExceptionInfo as normal deftype (to get a template)
2) then we remember reference to ExceptionInfo in ExceptionInfoTypeTemplate
3) then we redefine ExceptionInfo with raw constructor function which should mimic original behaviour (by scraping newly created js/Error instance, but calling ExceptionInfoTypeTemplate to do proper deftype initialization)
4) then we copy keys from ExceptionInfoTypeTemplate over ExceptionInfo
5) then we set ExceptionInfo's prototype to be ExceptionInfoTypeTemplate's prototype
6) then we fix ExceptionInfo's prototype's constructor to point to our re-defined constructor function
7) then we patch ExceptionInfo's prototype to inherit from js/Error (note this clobbers ExceptionInfoTypeTemplate as well - but we don't care about it)

This effectively gives us properly working ExceptionInfo deftype with redefined constructor function wrapping deftype's constructor for backwards compatibility. We also patch ExceptionInfo's prototype to inherit from js/Error the same was as original code did.

[1] https://github.com/binaryage/cljs-devtools/issues/23
[2] https://github.com/binaryage/cljs-devtools/releases/tag/v0.8.0

Currently ExceptionInfo is implemented as a raw constructor function which inherits from js/Error with some ad-hoc javascript-level patches to satisfy a tiny subset of deftype functionality (mainly for printing).

Unfortunately this does not play well with cljs-devtools[1]. This problem surfaced when I started playing with ExceptionInfo and cljs-devtools v0.8 which newly supports printing deftypes[2]. ExceptionInfo does not contain getBasis, cljs$lang$type, cljs$lang$ctorStr and similar machinery.

My proposed patch implements ExceptionInfo as a proper deftype and does some patch-work to provide backward compatibility. I'm pretty sure we must not break current contract of ExceptionInfo constructor accepting 3 args and synthesizing other fields on the fly in constructor.

Implementation details:
1) first we define ExceptionInfo as normal deftype (to get a template)
2) then we remember reference to ExceptionInfo in ExceptionInfoTypeTemplate
3) then we redefine ExceptionInfo with raw constructor function which should mimic original behaviour (by scraping newly created js/Error instance, but calling ExceptionInfoTypeTemplate to do proper deftype initialization)
4) then we copy keys from ExceptionInfoTypeTemplate over ExceptionInfo
5) then we set ExceptionInfo's prototype to be ExceptionInfoTypeTemplate's prototype
6) then we fix ExceptionInfo's prototype's constructor to point to our re-defined constructor function
7) then we patch ExceptionInfo's prototype to inherit from js/Error (note this clobbers ExceptionInfoTypeTemplate as well - but we don't care about it)

This effectively gives us properly working ExceptionInfo deftype with redefined constructor function wrapping deftype's constructor for backwards compatibility.
We also patch ExceptionInfo's prototype to inherit from js/Error the same was as original code did.

Note: With working deftype, we can move IPrintWithWriter and toString implementation to deftype itself.

[1] https://github.com/binaryage/cljs-devtools/issues/23
[2] https://github.com/binaryage/cljs-devtools/releases/tag/v0.8.0
Antonin Hildebrand made changes -
Attachment CLJS-1722.patch [ 15808 ]
Antonin Hildebrand made changes -
Attachment CLJS-1722.patch [ 15807 ]
Antonin Hildebrand made changes -
Description Currently ExceptionInfo is implemented as a raw constructor function which inherits from js/Error with some ad-hoc javascript-level patches to satisfy a tiny subset of deftype functionality (mainly for printing).

Unfortunately this does not play well with cljs-devtools[1]. This problem surfaced when I started playing with ExceptionInfo and cljs-devtools v0.8 which newly supports printing deftypes[2]. ExceptionInfo does not contain getBasis, cljs$lang$type, cljs$lang$ctorStr and similar machinery.

My proposed patch implements ExceptionInfo as a proper deftype and does some patch-work to provide backward compatibility. I'm pretty sure we must not break current contract of ExceptionInfo constructor accepting 3 args and synthesizing other fields on the fly in constructor.

Implementation details:
1) first we define ExceptionInfo as normal deftype (to get a template)
2) then we remember reference to ExceptionInfo in ExceptionInfoTypeTemplate
3) then we redefine ExceptionInfo with raw constructor function which should mimic original behaviour (by scraping newly created js/Error instance, but calling ExceptionInfoTypeTemplate to do proper deftype initialization)
4) then we copy keys from ExceptionInfoTypeTemplate over ExceptionInfo
5) then we set ExceptionInfo's prototype to be ExceptionInfoTypeTemplate's prototype
6) then we fix ExceptionInfo's prototype's constructor to point to our re-defined constructor function
7) then we patch ExceptionInfo's prototype to inherit from js/Error (note this clobbers ExceptionInfoTypeTemplate as well - but we don't care about it)

This effectively gives us properly working ExceptionInfo deftype with redefined constructor function wrapping deftype's constructor for backwards compatibility.
We also patch ExceptionInfo's prototype to inherit from js/Error the same was as original code did.

Note: With working deftype, we can move IPrintWithWriter and toString implementation to deftype itself.

[1] https://github.com/binaryage/cljs-devtools/issues/23
[2] https://github.com/binaryage/cljs-devtools/releases/tag/v0.8.0
Currently ExceptionInfo is implemented as a raw constructor function which inherits from js/Error with some ad-hoc javascript-level patches to satisfy a tiny subset of deftype functionality (mainly for printing).

Unfortunately this does not play well with cljs-devtools[1]. This problem surfaced when I started playing with ExceptionInfo and cljs-devtools v0.8 which newly supports printing deftypes[2]. ExceptionInfo does not contain getBasis, cljs$lang$type, cljs$lang$ctorStr and similar machinery.

My proposed patch implements ExceptionInfo as a proper deftype and does some patch-work to provide backward compatibility. I'm pretty sure we must not break current contract of ExceptionInfo constructor accepting 3 args and synthesizing other fields on the fly in constructor.

Implementation details:
1) first we define ExceptionInfo as normal deftype (to get a template)
2) then we remember reference to ExceptionInfo in ExceptionInfoTypeTemplate
3) then we redefine ExceptionInfo with raw constructor function which should mimic original behaviour (by scraping newly created js/Error instance, but calling ExceptionInfoTypeTemplate to do proper deftype initialization)
4) then we copy keys from ExceptionInfoTypeTemplate over ExceptionInfo
5) then we set ExceptionInfo's prototype to be ExceptionInfoTypeTemplate's prototype
6) then we fix ExceptionInfo's prototype's constructor to point to our re-defined constructor function
7) then we patch ExceptionInfo's prototype to inherit from js/Error (note this clobbers ExceptionInfoTypeTemplate as well - but we don't care about it)

This effectively gives us properly working ExceptionInfo deftype with redefined constructor function wrapping deftype's constructor for backwards compatibility.
We also patch ExceptionInfo's prototype to inherit from js/Error the same was as original code did.

Note: With working deftype, we can move IPrintWithWriter and toString implementation to the deftype itself.

[1] https://github.com/binaryage/cljs-devtools/issues/23
[2] https://github.com/binaryage/cljs-devtools/releases/tag/v0.8.0
Antonin Hildebrand made changes -
Patch Code [ 10001 ]

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated: