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

Variable initialization do not eval last expression #3

Conversation

VincentDamour
Copy link

Fix for this issue

With this PR, variable initialization will not be push into the expressions array.
Inline variable initialization with an output is still valid. let x = 1; x will output 1

@VincentDamour VincentDamour force-pushed the fix-last-exp-eval-twice branch from f786a30 to ff49011 Compare July 9, 2016 17:05
@StephenGrider
Copy link
Owner

@funny-vince Thanks for the contribution, this change would be a solid improvement.

One small issue: while the statement

const number = 1; 
number;

Does print '1', it appears that the nearly equivalent expression

const numbers = [1,2,3]; 
numbers;

Will not print anything, even though

const numbers = [1,2,3]; numbers;

Does give the expected result.

The 'eval' function tends to return some unpredictable results at times, it may be worth confirming that this behavior isn't being caused by eval.

@VincentDamour
Copy link
Author

@StephenGrider Hi, thanks for your answer. Fixing the issue you have mentioned will break the evaluation of loop with variable initialization in their declaration. For now I cannot help you, I'm sorry for this. Your code works great with eval(), but I found many small bugs while trying to fix this issue. I don't know what are you plan and your goal for JSPlaygrounds, but if you want a neat playground (like JS Bin and JSFiddle) there is probably many things to think about and a lot of code to change/write. If you need help with this, I will be more than happy to help you, but for now, I cannot fix this issue without changing a lot of the existing code.

@StephenGrider
Copy link
Owner

no problem

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.

2 participants