Set the context of executable class body wrappers #4828
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
|
LGTM. |
Collaborator
Author
|
Any other notes? I would quite like to get this and #4826 into a (patch) release. |
connec
added a commit
to connec/yaml-js
that referenced
this pull request
Dec 18, 2017
There were 4 sources of breakages: - `super` must be called in extended classes with constructors. - Class prototype methods are non-enumerable, so `util.extend` had to be updated to use `Object.getOwnPropertyNames` and `Object.getPrototypeOf`. - Default arguments will only be used for `undefined` values, which broke a test setup in `test/test.coffee`. - Due to `super` needing to be called before `@` parameters are availble, the stack set by `YAMLError#constructor` was calling `#toString` of subclasses whose `@` parameters were not yet available. The chosen fix was to make `#stack` a getter. This revealed a bug in CS which has been reported as jashkenas/coffeescript#4827. For now the CS dependency has been pinned to a proposed fix for that issue (jashkenas/coffeescript#4828).
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #4827.
I did first try to use an arrow function by setting
bound = trueon the wrapper, but this messed with the handling of@within the class body, where it should refer to the class being defined.We could opt to perform the binding only if the class
extendsan@-value but that would add a bit of extra complexity in the compiler for equivalent and, imo, no less readable output.