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

Added a transpile-only ESM loader (#1101) #1102

Merged
merged 7 commits into from
Aug 12, 2020
Merged

Added a transpile-only ESM loader (#1101) #1102

merged 7 commits into from
Aug 12, 2020

Conversation

concision
Copy link
Contributor

This resolves the feature request #1101: A new ESM loader equivalent of 'ts-node/register/transpile-only'

@codecov
Copy link

codecov bot commented Aug 10, 2020

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1102   +/-   ##
=======================================
  Coverage   75.49%   75.49%           
=======================================
  Files           7        7           
  Lines         657      657           
  Branches      148      148           
=======================================
  Hits          496      496           
  Misses        107      107           
  Partials       54       54           
Flag Coverage Δ
#node_10 72.15% <ø> (ø)
#node_12_15 72.53% <ø> (ø)
#node_12_16 72.53% <ø> (ø)
#node_13 75.19% <ø> (ø)
#node_14 75.19% <ø> (ø)
#typescript_2_7 75.03% <ø> (ø)
#typescript_latest 74.27% <ø> (ø)
#typescript_next 74.12% <ø> (ø)
#ubuntu 75.19% <ø> (ø)
#windows 75.34% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 ce7c323...525b730. Read the comment docs.

@coveralls
Copy link

coveralls commented Aug 10, 2020

Coverage Status

Coverage increased (+1.0%) to 83.175% when pulling 525b730 on concision:ab/1101 into ce7c323 on TypeStrong:master.

@cspotcode
Copy link
Collaborator

Looks good but it'll need tests before being merged, and our package.json will need to be updated.

You can probably copy-paste an existing ESM test, but add a deliberate type error to make sure it gets ignored.

We also have tests that each entrypoint can be resolved, which will validate the changes to package.json.

Copy link
Collaborator

@cspotcode cspotcode left a comment

Choose a reason for hiding this comment

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

Needs package.json updates and tests; otherwise looks good.

@concision
Copy link
Contributor Author

I have added esm/transpile-only.mjs to the package.json exports and corresponding resolution tests.

I also added two tests to verify the loader should behave as expected:

  • verify that the transpile-only ESM loader skips type checks in the presence of type errors
  • verify that the transpile-only ESM loader throws a type error in the presence of type errors

Are there any other needed changes I need to make?

Copy link
Collaborator

@cspotcode cspotcode left a comment

Choose a reason for hiding this comment

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

Looks great; just a couple questions.

tests/esm-transpile-only/index.ts Outdated Show resolved Hide resolved
src/index.spec.ts Show resolved Hide resolved
@cspotcode cspotcode merged commit e29ae04 into TypeStrong:master Aug 12, 2020
@cspotcode
Copy link
Collaborator

@concision looks great, thanks. It's merged. I need a review of #970 before making our next release, but hopefully we can get that done this weekend. It'll be 9.0.0

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