Skip to content

feat: add missing AST nodes to FunctionlessAST for 1:1 parity with TS#347

Merged
sam-goodwin merged 30 commits intomainfrom
samg-one-to-one-ast
Jul 27, 2022
Merged

feat: add missing AST nodes to FunctionlessAST for 1:1 parity with TS#347
sam-goodwin merged 30 commits intomainfrom
samg-one-to-one-ast

Conversation

@sam-goodwin
Copy link
Copy Markdown
Owner

@sam-goodwin sam-goodwin commented Jul 22, 2022

This change will bring 1:1 parity of the FunctionlessAST with the TypeScript AST so that our annotation process can support all valid syntax. Interpreters can still ignore unsupported statements, but we should be able to totally represent programs with our AST.

BREAKING CHANGE: FunctionlessAST contract changed to be 1:1 with ES AST.

@sam-goodwin sam-goodwin requested a review from thantos July 22, 2022 04:17
@netlify
Copy link
Copy Markdown

netlify bot commented Jul 22, 2022

Deploy Preview for effortless-malabi-1c3e77 ready!

Name Link
🔨 Latest commit 360586f
🔍 Latest deploy log https://app.netlify.com/sites/effortless-malabi-1c3e77/deploys/62e17b0432eb170008e63c9f
😎 Deploy Preview https://deploy-preview-347--effortless-malabi-1c3e77.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sam-goodwin sam-goodwin marked this pull request as ready for review July 22, 2022 18:01
@sam-goodwin sam-goodwin requested a review from thantos July 22, 2022 20:10
@sam-goodwin sam-goodwin self-assigned this Jul 22, 2022
@sam-goodwin sam-goodwin requested a review from thantos July 27, 2022 03:09
Comment on lines +72 to +74
for (const arg of args) {
setParent(arg);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: setParent(arg) (if IArguments is an array, it will just recurse)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I don't understand, what's wrong?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nothing wrong, just saying your function also supports arrays of arguments so it could be written as:

const setParent = (node: any) => {
      if (!node) {
        return;
      } else if (isNode(node)) {
        node.setParent(this as FunctionlessNode);
      } else if (Array.isArray(node)) {
        node.forEach(setParent);
      }
    };
setParent(args);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Actually, IArguments is not an array, it is only Iterable

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Let me take another look, perhaps I got confused by a type error. I do remember doing it your way to begin with

@sam-goodwin sam-goodwin requested a review from thantos July 27, 2022 06:23
Comment on lines +72 to +74
for (const arg of args) {
setParent(arg);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nothing wrong, just saying your function also supports arrays of arguments so it could be written as:

const setParent = (node: any) => {
      if (!node) {
        return;
      } else if (isNode(node)) {
        node.setParent(this as FunctionlessNode);
      } else if (Array.isArray(node)) {
        node.forEach(setParent);
      }
    };
setParent(args);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants