Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CS1] fix #4260 and #1349: splat error with soak properties or expressions #4643

Merged
merged 4 commits into from
Aug 27, 2017

Conversation

zdenko
Copy link
Collaborator

@zdenko zdenko commented Aug 16, 2017

This is a fix for #4260 and #1349 when invalid JS is compiled if Splat contains soak accessor (?.), or if statement.

Before:

// [a if b...]
slice.call(  if (b) {
    a;
  });

// [a?.b...]
slice.call(  if (typeof a !== "undefined" && a !== null) {
    a.b;
  });

// arr.push(error.data?.errors...)
arr.push.apply(arr, if ((ref = error.data) != null) {
  ref.errors;
});

After:

// [a if b...]
slice.call((b ? a : void 0));

// [a?.b...]
slice.call((typeof a !== "undefined" && a !== null ? a.b : void 0));

// arr.push(error.data?.errors...)
arr.push.apply(arr, ((ref = error.data) != null ? ref.errors : void 0));

@GeoffreyBooth
Copy link
Collaborator

See #4644. I’m not sure this bug is severe enough that it’s worth fixing in 1.x. Since the current output throws runtime errors, this bug doesn’t cause any bugs in anyone’s code; it’s just that some syntax that should be allowable just isn’t, until #4644 gets merged in. Or to look at it another way, we add support for this syntax in CoffeeScript 2 😄

@zdenko
Copy link
Collaborator Author

zdenko commented Aug 23, 2017

@GeoffreyBooth void 0 is the problem. When the slice is called the correct output should be:

// [a?.b...]
slice.call((typeof a !== "undefined" && a !== null ? a.b : []));

I'll take a look.

@zdenko
Copy link
Collaborator Author

zdenko commented Aug 23, 2017

@GeoffreyBooth I think this should be fixed now.

@@ -1812,7 +1812,7 @@ exports.Splat = class Splat extends Base
assigns: (name) ->
@name.assigns name

compileToFragments: (o) ->
compileNode: (o) ->
@name.compileToFragments o
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this get LEVEL_OP too?

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this case no, since in CS1 splats are not operators, but are instead handled by Arr and Assign.

@GeoffreyBooth GeoffreyBooth merged commit 6cea181 into jashkenas:master Aug 27, 2017
@GeoffreyBooth GeoffreyBooth deleted the soak-splat-fix-cs1 branch August 27, 2017 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants