Skip to content
This repository was archived by the owner on Jan 14, 2019. It is now read-only.

feat: typed parser return value #33

Merged
merged 5 commits into from
Nov 26, 2018
Merged

feat: typed parser return value #33

merged 5 commits into from
Nov 26, 2018

Conversation

g-plane
Copy link
Contributor

@g-plane g-plane commented Nov 10, 2018

Previously, type of the return value of parse function is any. This PR makes it strongly typed.

Additions:

  • The options can be undefined now and all the properties of options can be undefined. That is, all the code below are valid:
// We can omit the options:
parse('source code')

// All properties are optional:
parse('source code', {})
  • The properties of the parser result are dynamic. For example, if we disable the tokens option like this:
let ast = parse('', { tokens: false })

The tokens property will not exist in the variable ast.

default

Instead, if we enable the tokens option:

let ast = parse('', { tokens: true })

default

We can find that an additional tokens property is existed on the ast.

@JamesHenry
Copy link
Owner

Great addition, @g-plane! I had to prioritize getting in the big PR from Ben, would you mind updating this?

Thanks!

@g-plane
Copy link
Contributor Author

g-plane commented Nov 22, 2018

What does it need to be updated?

@j-f1
Copy link
Contributor

j-f1 commented Nov 22, 2018

@g-plane It looks like there are merge conflicts now.

@g-plane
Copy link
Contributor Author

g-plane commented Nov 22, 2018

Oh, I'm using mobile phone now so I cannot see the conflicts notice. Sorry!

@j-f1
Copy link
Contributor

j-f1 commented Nov 22, 2018

You should try GitHawk! </ad>

Sent with GitHawk

@g-plane
Copy link
Contributor Author

g-plane commented Nov 23, 2018

@JamesHenry Problems are fixed.

src/parser.ts Outdated
(currentProgram: ts.Program) => {
const ast = currentProgram.getSourceFile(options.filePath);
const ast = currentProgram.getSourceFile(
options.filePath || (options.jsx ? 'estree.ts' : 'estree.tsx')
Copy link
Owner

Choose a reason for hiding this comment

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

This filename logic is now separately defined 3 times within this file, can we promote it to be defined once at a higher level?

Other than that the PR LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@JamesHenry
Copy link
Owner

Thanks!

@JamesHenry JamesHenry merged commit 7599aba into JamesHenry:master Nov 26, 2018
@g-plane g-plane deleted the typed-return-value branch November 26, 2018 02:22
@JamesHenry
Copy link
Owner

🎉 This PR is included in version 5.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants