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

fixed multiline decision for object patterns with rest elements #408

Conversation

sanex3339
Copy link
Contributor

Fixed error during code generation for object pattern single rest-element property

/node_modules/escodegen/escodegen.js:2226
                if (property.value.type !== Syntax.Identifier) {
                                   ^
TypeError: Cannot read property 'type' of undefined

@sanex3339
Copy link
Contributor Author

@michaelficarra will happy if you check it

@sanex3339
Copy link
Contributor Author

@michaelficarra please check this. 2 months left since PR has created. I need fix for this, because my tool should support this syntax.

@sanex3339
Copy link
Contributor Author

Related issue
javascript-obfuscator/javascript-obfuscator#607

@sanex3339
Copy link
Contributor Author

Also, I think, this package needs more active maintainers.

test/harmony.js Outdated Show resolved Hide resolved
@sanex3339
Copy link
Contributor Author

@michaelficarra updated

Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

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

I think this problem is actually a little more complicated. The whole multiline determination just doesn't seem to account for RestElement. If you see below, property.shorthand is being tested without first checking property.type. I think we need to add a test there as well and a test here for property.argument being an Identifier if property.type is RestElement.

@sanex3339
Copy link
Contributor Author

sanex3339 commented Jun 2, 2020

@michaelficarra
478df26

Can you show me case when RestElement's argument can be different than Identifier inside ObjectPattern?

@takahser
Copy link

takahser commented Jun 3, 2020

When will this be fixed? I experience the same problem atm.
@sanex3339 idk where in my code the problem occurs, but with this fix it doesn't. It's an angular app in my case.

@takahser
Copy link

takahser commented Jun 3, 2020

For me it's exactly this issue as well: javascript-obfuscator/javascript-obfuscator#607

Would be great if this could be merged soon @michaelficarra

@michaelficarra
Copy link
Member

@sanex3339 You're right, that should be sufficient now. Thanks for the contribution.

@michaelficarra michaelficarra changed the title Fixed error during code generation for object pattern single rest-element property fixed multiline decision for object patterns with rest elements Jun 3, 2020
@michaelficarra michaelficarra merged commit c62cbe2 into estools:master Jun 3, 2020
@takahser
Copy link

takahser commented Jun 3, 2020

thx a lot @michaelficarra 👍
it would be great if you could make a patch release on npm as well 👍

@sanex3339
Copy link
Contributor Author

Thank you!

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