-
Notifications
You must be signed in to change notification settings - Fork 14
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
fix: appsync and step function visitor hoist refactor #280
Conversation
✅ Deploy Preview for effortless-malabi-1c3e77 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
src/asl.ts
Outdated
return taskState; | ||
throw Error(""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should error message be? Why did the logic change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got rid of findIntegration
which will align better with validating and handling the async change. Will fix error.
*/ | ||
public generateOrGet(node: FunctionlessNode): string { | ||
if (!this.generatedNames.has(node)) { | ||
this.generatedNames.set(node, `${this.generatedNames.size}_tmp`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look at who is using mutability ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied your code ;-) but in this case the risk seemed low in a small isolated private methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied your code ;-)
Loool
src/visit.ts
Outdated
): BlockStmt { | ||
return visitEachChild(block, (stmt) => { | ||
const nestedTasks: FunctionlessNode[] = []; | ||
function hoist(expr: Expr): Expr { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the return type be an Identifier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh, yeah, it could be, it was more complex at one point.
/** | ||
* Like {@link visitEachChild} but it only visits the statements of a block. | ||
* | ||
* Provides the hoist function that allows hoisting expressions into variable statements above the current statement. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty nice I must say. Neat.
Signed-off-by: github-actions <github-actions@github.com>
Signed-off-by: github-actions <github-actions@github.com>
…ionless into appsync_visit
Signed-off-by: github-actions <github-actions@github.com>
Closes #212
Refactor step functions and app sync interpreters to refactor nested tasks to a normal form. This change will make the #202 easier by providing a generic interface for re-writing the AST tree. Async introduces more complex statements that need to be hoisted to a-normal form which would have been previously difficult.