Skip to content
This repository has been archived by the owner on Feb 7, 2021. It is now read-only.

feat: update object-to-ast.js and helpers.js to TS #4

Merged
merged 6 commits into from
Feb 3, 2021
Merged

Conversation

mpeyper
Copy link
Member

@mpeyper mpeyper commented Feb 3, 2021

This is what I have so far. There are some issues which I'll comment in the files.

Fixes #3
Fixes #6

src/object-to-ast.ts Outdated Show resolved Hide resolved
src/object-to-ast.ts Outdated Show resolved Hide resolved
@mpeyper mpeyper changed the title feat: update object-to-ast to TS (work-in-progress) feat: update object-to-ast to TS Feb 3, 2021
const stringified = stringify(object)
const variableDeclarationNode = babel.template(`var x = ${stringified}`, {
preserveComments: true,
placeholderPattern: false,
...fileOptions.parserOpts,
sourceType: 'module',
})()
})() as babelCore.types.VariableDeclaration
Copy link
Member Author

Choose a reason for hiding this comment

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

I worked out where declarations is defined and it makes sens based on what is being passed the babel.template() (i.e. a declaration of var x ...).

I went for a straight up cast here given the template is essentially hard coded to be a VariableDeclaration, but I've got the voice in the back of my head telling me this is wrong and I should use a type guard.

This type guard works:

function isVariableDeclaration(node: babelCore.types.Statement | babelCore.types.Statement[]): node is babelCore.types.VariableDeclaration {
  return !Array.isArray(node) && node.type === 'VariableDeclaration'
}

However, I'm not sure what code to put on the failing side of it:

  if (isVariableDeclaration(variableDeclarationNode)) {
    return variableDeclarationNode.declarations[0].init // knows the type now without casting :)
  }

  // what now? Error? You can't get here...

@mpeyper mpeyper marked this pull request as ready for review February 3, 2021 11:35
Copy link

@datner datner left a comment

Choose a reason for hiding this comment

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

LGTM, but I really don't like the casting..........

package.json Outdated Show resolved Hide resolved
src/object-to-ast.ts Outdated Show resolved Hide resolved
src/helpers.ts Show resolved Hide resolved
src/helpers.ts Show resolved Hide resolved
@mpeyper mpeyper changed the title feat: update object-to-ast to TS feat: update object-to-ast.js and helpers.js to TS Feb 3, 2021
@JacobMGEvans JacobMGEvans merged commit 2062a24 into main Feb 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate helpers.js to TS Migrate object-to-ast.js to TS
6 participants