Skip to content

Conversation

@sabakugaara
Copy link

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

@sabakugaara
Copy link
Author

code from: #21, fix lint error

@codecov
Copy link

codecov bot commented Jan 18, 2020

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.75%. Comparing base (85a0098) to head (3f51e27).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #23   +/-   ##
=======================================
  Coverage   96.75%   96.75%           
=======================================
  Files           5        5           
  Lines         185      185           
=======================================
  Hits          179      179           
  Misses          6        6           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sabakugaara sabakugaara mentioned this pull request Jan 18, 2020
4 tasks
@atian25 atian25 requested a review from whxaxes January 21, 2020 06:28
@atian25
Copy link
Member

atian25 commented Jan 21, 2020

这 2 个 PR 的关系是?

那边还有几个 Review 问题还没解决

@sabakugaara
Copy link
Author

这 2 个 PR 的关系是?

那边还有几个 Review 问题还没解决

代码是一样的,只有一个 lint 问题修复了,但是看原作者好像没打算继续更新。
所以,建议在这个分支上继续

@atian25
Copy link
Member

atian25 commented Jan 22, 2020

@whxaxes 看下

@sabakugaara
Copy link
Author

ping @whxaxes

@whxaxes
Copy link
Member

whxaxes commented Mar 10, 2020

补一下单测

}

if (stat.isFile() && path.extname(name) === '.js') {
if (stat.isFile() && [ '.js', '.ts' ].includes(path.extname(name))) {
Copy link
Member

Choose a reason for hiding this comment

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

这里感觉有问题,如果项目下刚好同时存在同名的 .js 和 .ts 文件,但是又没有使用 ts-node ,下面的 require 就会出问题

Copy link
Author

@sabakugaara sabakugaara Apr 23, 2020

Choose a reason for hiding this comment

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

有改法建议吗?@whxaxes


if (stat.isFile() && path.extname(name) === '.js') {
if (stat.isFile() && [ '.js', '.ts' ].includes(path.extname(name))) {
let handler = require(filepath);
Copy link
Member

Choose a reason for hiding this comment

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

这里直接 require 也会有问题,ts compile 出来的 js 是 esm ,模块内容是挂在 default 中的

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