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

Fix error when Typescript is not installed #282

Merged
merged 2 commits into from
Aug 23, 2018
Merged

Fix error when Typescript is not installed #282

merged 2 commits into from
Aug 23, 2018

Conversation

mnkhouri
Copy link
Member

Fixes #280. The code was referencing the typescript object when it
is potentially null.

It's not included in this PR, but we could consider adding a check to
catch this situation in CI (filed as an issue in #281).

Fixes #280. The code was referencing the `typescript` object when it
is potentially `null`.
@mnkhouri mnkhouri requested a review from LinusU August 22, 2018 02:11
@codecov
Copy link

codecov bot commented Aug 22, 2018

Codecov Report

Merging #282 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #282   +/-   ##
=======================================
  Coverage   98.04%   98.04%           
=======================================
  Files          29       29           
  Lines         461      461           
=======================================
  Hits          452      452           
  Misses          9        9
Impacted Files Coverage Δ
src/parser/typescript.js 83.33% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96161f9...d3d899e. Read the comment docs.

@mnkhouri mnkhouri mentioned this pull request Aug 22, 2018
5 tasks
Copy link
Contributor

@nkbt nkbt left a comment

Choose a reason for hiding this comment

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

Nice one

@mnkhouri
Copy link
Member Author

@lijunle or @LinusU, if you have a chance to review this PR, it'll allow us to publish 0.6.10 :)

Copy link
Member

@lijunle lijunle left a comment

Choose a reason for hiding this comment

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

Looks good!

@lijunle
Copy link
Member

lijunle commented Aug 23, 2018

It's not included in this PR, but we could consider adding a check to catch this situation in CI

This one is hard. If the environment is complicated and stability is low, it may be not worth.

@mnkhouri mnkhouri merged commit 0a11cf8 into master Aug 23, 2018
@mnkhouri mnkhouri deleted the fix_ts_import branch August 23, 2018 16:13
mnkhouri added a commit that referenced this pull request Aug 28, 2018
* master:
  Upgrade all dependencies (#276)
  Fix error when Typescript is not installed (#282)
  Enable ES6/7 syntax in Typescript (#258)
  Update NPM tokens for deploy from Travis (#278)
  Recognize object arrays in Webpack module.rules.loaders (#233)
  Add support for import() expressions (#205)
  Bump Mocha to 5.x (#274)
  Fix ignore-bin-package default in ReadMe (#252)
  Fix typo: pasers -> parsers
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.

3 participants