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: typescript definition #73

Closed
wants to merge 1 commit into from
Closed

fix: typescript definition #73

wants to merge 1 commit into from

Conversation

yenshih
Copy link

@yenshih yenshih commented Oct 22, 2019

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

Better TypeScript definition base on control flow analysis for assertions:
microsoft/TypeScript#32695

const loader: Loader = function(source) {
    const options = getOptions(this) || {};

    // typeof options --> OptionObject

    validate<SpecificLoaderOptions>(schema, options);

    // typeof options --> SpecificLoaderOptions
};

Breaking Changes

Additional Info

@codecov
Copy link

codecov bot commented Oct 22, 2019

Codecov Report

Merging #73 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #73   +/-   ##
=======================================
  Coverage   98.59%   98.59%           
=======================================
  Files           5        5           
  Lines         568      568           
  Branches      255      255           
=======================================
  Hits          560      560           
  Misses          8        8

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 f38158d...b30f184. Read the comment docs.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

/cc @vankop Can you look?

@alexander-akait
Copy link
Member

Oh, it is only for typescript@3.7 (not released yet)

@vankop
Copy link
Member

vankop commented Oct 24, 2019

this is great approach, but as @evilebottnawi said already it is for future typescript release

@yenshih
Copy link
Author

yenshih commented Oct 25, 2019

Yes, typescript@3.7 will be released in 2 weeks. We need to prepare for that. I'm using typescript@rc for now.
microsoft/TypeScript#33352

@yenshih
Copy link
Author

yenshih commented Nov 6, 2019

@vankop
Copy link
Member

vankop commented Nov 6, 2019

declaration now works with JS files 🤩 PR to core webpack confirmed =)

@alexander-akait
Copy link
Member

/cc @yenshih can you put this using JSDoc? Now we generate declarations from JSDoc, anyway thanks for the PR

@yenshih
Copy link
Author

yenshih commented Nov 8, 2019

It's fine :)
However, asserts is an advanced feature of TypeScript. I have no idea how to use asserts in JSDoc.

@yenshih yenshih closed this Nov 8, 2019
@yenshih yenshih deleted the typescript-definition branch November 8, 2019 02:00
@vankop
Copy link
Member

vankop commented Nov 8, 2019

It works same way as in ts files

@alexander-akait
Copy link
Member

@vankop can you take care?

@vankop
Copy link
Member

vankop commented Nov 8, 2019

@evilebottnawi later, maybe on the weekend

@alexander-akait alexander-akait mentioned this pull request Nov 12, 2019
6 tasks
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