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: fix source map line number incorrect #107

Merged
merged 11 commits into from
Aug 27, 2018
Merged

Conversation

whxaxes
Copy link
Member

@whxaxes whxaxes commented Aug 24, 2018

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

修复跑单测时的堆栈行数及列数错误问题。

原因

espower-typescript 里会在 ts 的 sourcemap 基础上再加上 power-assert 的 sourcemap,因为 power-assert 会对源码做处理,然后会返回一个新的 code + sourcemap

然后 ts-node 里面自己内置了 source-map-support ,然后在 source-map-support 拿 code 的时候命中了 ts-node 的缓存 ( https://github.com/TypeStrong/ts-node/blob/master/src/index.ts#L218 ),拿到的是 ts-node 编译后的源码,而不是上面 espower-typescript 处理返回的...

所以导致 source-map-support 在实际处理 source map 映射的时候,code 是 power-assert 编译后的代码,sourceMap 却是 ts 的 sourceMap....就导致了这个问题。

解决方案

在 egg-bin test 的时候,由 egg-bin 来注册 source-map-support,缓存解析后的代码并且传给 sourceMapSupport。


Fix bug that error stack gives incorrect line number and column number in unit testing.

Reason

espower-typescript combines the sourceMap of ts-node and power-assert and return a new sourceMap for supporting power-assert. But source-map-support receives old sourceMap when handling the code because ts-node cache its sourceMap in retrieveFile method: https://github.com/TypeStrong/ts-node/blob/master/src/index.ts#L218

// Install source map support and read from memory cache.
sourceMapSupport.install({
  environment: 'node',
  retrieveFile: function (path) {
      return memoryCache.outputs[path];
  }
});

Solution

Overwriting the retrieveFile method of source-map-support in egg-bin test to return a correct sourceMap for code.

power-assert-js/espower-typescript#54

@whxaxes whxaxes requested review from atian25 and popomore August 24, 2018 12:43
@whxaxes
Copy link
Member Author

whxaxes commented Aug 24, 2018

ci 挂了是因为刚发的 egg-mock 声明有问题,已经提 PR : eggjs/egg-mock#86

@@ -105,6 +105,9 @@ class TestCommand extends Command {
if (testArgv.typescript) {
// remove ts-node in context getter on top.
Copy link
Member

Choose a reason for hiding this comment

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

这行注释跟下面对不上?

Copy link
Member Author

Choose a reason for hiding this comment

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

是说明一下在上面的代码中把 ts-node 删掉了...

@@ -105,6 +105,9 @@ class TestCommand extends Command {
if (testArgv.typescript) {
// remove ts-node in context getter on top.
requireArr.push(require.resolve('espower-typescript/guess'));
Copy link
Member

Choose a reason for hiding this comment

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

这个要不要放到下面那个文件里面去引入?

下面那个改名为 typescript-test-support.js 啥的

Copy link
Member Author

@whxaxes whxaxes Aug 24, 2018

Choose a reason for hiding this comment

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

还是分开好一些,两者的功能不一样,分开的逻辑会更清晰一些

},
});

extensions.forEach(ext => {
Copy link
Member

Choose a reason for hiding this comment

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

开发者如果不用 ts 而是用 babel/register 会不会也有类似问题?

Copy link
Member

Choose a reason for hiding this comment

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

for of 吧

Copy link
Member Author

Choose a reason for hiding this comment

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

得看 babel/register 里有没有内置 source map support 并且缓存文件了

const cacheMap = {};
const extensions = [ '.ts', '.tsx' ];

sourceMapSupport.install({
Copy link
Member

Choose a reason for hiding this comment

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

egg-scripts 那边不受影响吧?

Copy link
Member Author

Choose a reason for hiding this comment

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

不受,只有用了 espower-typescript 才有这个问题

@codecov
Copy link

codecov bot commented Aug 24, 2018

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #107   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          11     12    +1     
  Lines         206    220   +14     
=====================================
+ Hits          206    220   +14
Impacted Files Coverage Δ
lib/cmd/test.js 100% <100%> (ø) ⬆️
lib/correct-source-map.js 100% <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 e35b346...c71c3f2. Read the comment docs.

Copy link
Member

@atian25 atian25 left a comment

Choose a reason for hiding this comment

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

+1

sourcemap 那里加下英文注释下吧

@atian25
Copy link
Member

atian25 commented Aug 27, 2018

我指的是这个 PR 里面描述那一堆,简单用英文描述下他们的冲突情况和解决方式吧

@whxaxes
Copy link
Member Author

whxaxes commented Aug 27, 2018

@atian25 补上了

@atian25
Copy link
Member

atian25 commented Aug 27, 2018

ci fail

@popomore popomore merged commit ca4f78f into master Aug 27, 2018
@popomore popomore deleted the fix-source-map-incorrect branch August 27, 2018 10:51
@popomore
Copy link
Member

fengmk2 pushed a commit that referenced this pull request Dec 17, 2022
[skip ci]

## 1.0.0 (2022-12-17)

### Features

* add `egg-bin autod --check` command ([#70](#70)) ([0f0c7ab](0f0c7ab))
* add COV_EXCLUDES for coverage excludes ([#7](#7)) ([a33e561](a33e561))
* add sticky mode support ([#32](#32)) ([f5a1152](f5a1152))
* add ts env in command ([#98](#98)) ([756310e](756310e))
* allow loading ts compiler from cwd ([#169](#169)) ([4a54cec](4a54cec))
* auto detect available port ([#22](#22)) ([6d7ad41](6d7ad41))
* auto require setup file ([#24](#24)) ([a847750](a847750))
* build-in intelli-espower-loader ([#20](#20)) ([4e86a67](4e86a67))
* change default timeout to 60000 ([#50](#50)) ([cbc9263](cbc9263))
* commands support specific execArgv(harmony) ([#33](#33)) ([0c7108b](0c7108b))
* cov support output json-summary ([#64](#64)) ([c1de734](c1de734))
* cov support typescript ([#91](#91)) ([8a05e19](8a05e19))
* **cov:** add nyc instrument passthrough ([#103](#103)) ([8fa805b](8fa805b))
* **cov:** add prerequire option ([#53](#53)) ([055c47f](055c47f))
* **cov:** add prerequire option ([#53](#53)) ([a6a2b4a](a6a2b4a))
* debug proxy support ([678b83d](678b83d))
* **debug:** [BREAKING_CHANGE] remove iron-node ([#26](#26)) ([9d4170f](9d4170f))
* default enable c8 report ([#189](#189)) ([72e925b](72e925b))
* **dev:** pass debug args to execArgv ([#12](#12)) ([92b111f](92b111f))
* egg-bin check ([#87](#87)) ([92b1489](92b1489))
* enable mochawesome by default ([#193](#193)) ([6636e8f](6636e8f))
* expose proc ([#152](#152)) ([dcc9b25](dcc9b25))
* extractArgv refactor & extract debug port ([994616d](994616d))
* extractArgv support expose_debug_as ([6f5d525](6f5d525))
* impl parallel for mocha parallel mode ([#185](#185)) ([78141e8](78141e8))
* intergration with egg-ts-helper ([#123](#123)) ([263cfd1](263cfd1))
* pass --check to pkgfiles ([#48](#48)) ([7b0c995](7b0c995))
* remove correct-source-map.js ([#109](#109)) ([8b62742](8b62742))
* resolve istanbul path for coffee ([#9](#9)) ([147add8](147add8))
* revert egg-bin check ([#90](#90)) ([83322e6](83322e6))
* revert to 4.2.0 ([c65a00d](c65a00d))
* set EGG_MASTER_CLOSE_TIMEOUT when run dev ([#88](#88)) ([c48860c](c48860c))
* should print error stack ([#132](#132)) ([5c621f6](5c621f6))
* simplify mocha error stack ([#59](#59)) ([0b01158](0b01158))
* support $NODE_DEBUG_OPTION ([#71](#71)) ([1340ce7](1340ce7))
* support c8 report ([#172](#172)) ([1e96da2](1e96da2))
* support cov command  in win32 ([#52](#52)) ([93b731d](93b731d))
* support egg-bin test --changed ([#111](#111)) ([51f93aa](51f93aa))
* support egg.typescript ([#92](#92)) ([a7f0ca8](a7f0ca8))
* support mocha custom require args ([#5](#5)) ([fb8fd32](fb8fd32))
* support read egg.require from package.json ([#121](#121)) ([904103f](904103f))
* support set eggTsHelper ([#183](#183)) ([f564cbf](f564cbf))
* support switch ts compiler ([#158](#158)) ([a74bae2](a74bae2))
* support typescript ([#89](#89)) ([75b5cd6](75b5cd6))
* test  --dry-run ([#145](#145)) ([3cc3b0b](3cc3b0b))
* try to use --inspect first ([#19](#19)) ([d7ad24c](d7ad24c))
* update pkg.files that if file exists ([#37](#37)) ([af5af6a](af5af6a))
* upgrade espower-typescript to 9.0 ([#106](#106)) ([35e89db](35e89db))
* use nyc instead of istanbul ([#63](#63)) ([3cf312c](3cf312c))
* use test when run cov on Windows ([#18](#18)) ([611027f](611027f)), closes [/github.com/eggjs/egg/pull/133#issuecomment-256827488](https://github.com/eggjs//github.com/eggjs/egg/pull/133/issues/issuecomment-256827488)
* use unparseArgv from common-bin ([#45](#45)) ([da41b8e](da41b8e))

### Bug Fixes

* --full-trace should be boolean ([#191](#191)) ([bfd7fab](bfd7fab))
* -x only support string ([#47](#47)) ([b79f0cc](b79f0cc))
* .setup.js should be the first test file ([#30](#30)) ([4f45c6b](4f45c6b))
* add missing deps ([#42](#42)) ([86ebc75](86ebc75))
* add power-assert to deps ([#21](#21)) ([8b7ce8d](8b7ce8d))
* auto add .setup.ts file ([#147](#147)) ([00afdf7](00afdf7))
* can not find iron-node in subprocess ([#8](#8)) ([d6a57f5](d6a57f5))
* ci failed ([#162](#162)) ([4a076e6](4a076e6))
* clean more mocha error stack ([#60](#60)) ([9c77118](9c77118))
* conflix source map support ([#181](#181)) ([a1ec4f7](a1ec4f7))
* cov replaced warning at win ([#49](#49)) ([d2850a5](d2850a5))
* **cov:** istanbul path env ([#44](#44)) ([ce8f141](ce8f141))
* **cov:** wait 1 second for Windows ([#16](#16)) ([742f6fc](742f6fc))
* debug at 6.x ([469739f](469739f))
* debug mode detect ([#130](#130)) ([819d78f](819d78f))
* debug-test invoke formatTestArgs ([a82a87a](a82a87a))
* don't pass prerequire ([#57](#57)) ([e0d03d3](e0d03d3))
* downgrade ts-node ([#126](#126)) ([d802694](d802694))
* egginfo is not exists ([#159](#159)) ([8666e9e](8666e9e))
* ets not found ([#124](#124)) ([8f6135e](8f6135e))
* fix cov env ([#188](#188)) ([e18ceda](e18ceda))
* fix ENABLE_MOCHA_PARALLEL/AUTO_AGENT env ([#187](#187)) ([88ba6d5](88ba6d5))
* fix espwoer-typescript inject logic ([#178](#178)) ([2e0fecd](2e0fecd))
* fix source map line number incorrect ([#107](#107)) ([ca4f78f](ca4f78f)), closes [/github.com/TypeStrong/ts-node/blob/master/src/index.ts#L218](https://github.com/eggjs//github.com/TypeStrong/ts-node/blob/master/src/index.ts/issues/L218)
* fixed ts-node ignore files ([#105](#105)) ([a790304](a790304))
* force exit when runner complete ([#83](#83)) ([7386194](7386194))
* ignore eggTsHelper on node-test ([#186](#186)) ([c5db00e](c5db00e))
* ignore frontend files and document files ([#65](#65)) ([f9d578e](f9d578e))
* let sub class can override getFrameworkOrEggPath ([1320144](1320144))
* link mocha bin from inner file ([#15](#15)) ([f6cb195](f6cb195))
* make sure dev command eggPath can be override ([#23](#23)) ([22510d7](22510d7))
* make sure files sort in all platforms ([#86](#86)) ([6689962](6689962))
* nyc shim ([#138](#138)) ([3b370ef](3b370ef))
* only hotfix spawn-wrap on windows ([#69](#69)) ([e784a3d](e784a3d))
* package.json to reduce vulnerabilities ([#108](#108)) ([e910ae3](e910ae3))
* package.json to reduce vulnerabilities ([#81](#81)) ([e3c33e9](e3c33e9))
* remove espower typescript ([#160](#160)) ([563923a](563923a))
* remove temp excludes ([40aaca1](40aaca1))
* remove ts extensions by default ([#94](#94)) ([1c860a9](1c860a9))
* revert nyc ([#140](#140)) ([9cb8125](9cb8125)), closes [/github.com//pull/133#issuecomment-489387659](https://github.com/eggjs//github.com/eggjs/egg-bin/pull/133/issues/issuecomment-489387659)
* should exit when no test files found ([#100](#100)) ([e375ba4](e375ba4))
* should ignore fixtures and node_modules ([#96](#96)) ([e73c569](e73c569))
* should not timeout when debugging ([#129](#129)) ([3b6819c](3b6819c))
* should only read pkg if argv.typescript not pass ([#97](#97)) ([1e93020](1e93020))
* should pass customEgg to startCluster ([#25](#25)) ([e3d7974](e3d7974))
* should support -p ([#27](#27)) ([df86893](df86893))
* should support multi exclude dirs ([#66](#66)) ([1ac9d68](1ac9d68))
* support --workers same as egg-scripts ([#127](#127)) ([fcae123](fcae123))
* support egg-bin dev --cluster and --baseDir ([#36](#36)) ([ba7d409](ba7d409))
* support node4 ([#35](#35)) ([dbbaf98](dbbaf98))
* support relative path ([#95](#95)) ([7531faa](7531faa))
* There is a case sensitive issue from spawn-wrap  on Windows ([#67](#67)) ([56f8518](56f8518))
* use co-mocha instead of thunk-mocha ([#38](#38)) ([f6b5171](f6b5171))
* use context.env instead of process.env ([#58](#58)) ([9aa0030](9aa0030))
* use inspector at 7.x+ ([#74](#74)) ([2e3498e](2e3498e))
* using ts-node in app should check tscompiler and deps ([#170](#170)) ([662b9e9](662b9e9))
* wait more time for Window 😢 ([#17](#17)) ([d6f6b3b](d6f6b3b))
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