Skip to content

Commit

Permalink
fix: fix source map line number incorrect (#107)
Browse files Browse the repository at this point in the history
修复跑单测时的堆栈行数及列数错误问题。

### 原因

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

```typescript
// 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
  • Loading branch information
whxaxes authored and popomore committed Aug 27, 2018
1 parent e35b346 commit ca4f78f
Show file tree
Hide file tree
Showing 15 changed files with 171 additions and 3 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ test/fixtures/ts/node_modules/aliyun-egg/
!test/fixtures/test-files-glob/**
!test/fixtures/test-files-stack/node_modules/
!test/fixtures/example/node_modules/
!test/fixtures/example-ts-error-stack/node_modules/


**/run/*.json
.tmp
.vscode
.cache
*.log
package-lock.json
.nyc_output
3 changes: 3 additions & 0 deletions lib/cmd/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'));

// use to correct sourceMap mapping
requireArr.push(path.resolve(__dirname, '../correct-source-map'));
}

testArgv.require = requireArr;
Expand Down
55 changes: 55 additions & 0 deletions lib/correct-source-map.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* Only works in test/cov command with `espower-typescript`.
*
* 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
*
* ```typescript
* // 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.
*
*
* https://github.com/eggjs/egg-bin/pull/107
* https://github.com/power-assert-js/espower-typescript/issues/54
*
*/

'use strict';

const sourceMapSupport = require('source-map-support');
const cacheMap = {};
const extensions = [ '.ts', '.tsx' ];

sourceMapSupport.install({
environment: 'node',
retrieveFile(path) {
return cacheMap[path];
},
});

for (const ext of extensions) {
const originalExtension = require.extensions[ext];
require.extensions[ext] = (module, filePath) => {
const originalCompile = module._compile;
module._compile = function(code, filePath) {
cacheMap[filePath] = code;
return originalCompile.call(this, code, filePath);
};
return originalExtension(module, filePath);
};
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"globby": "^8.0.1",
"inspector-proxy": "^1.2.1",
"intelli-espower-loader": "^1.0.1",
"source-map-support": "^0.5.6",
"mocha": "^5.2.0",
"mz-modules": "^2.1.0",
"nyc": "^12.0.2",
Expand Down
9 changes: 9 additions & 0 deletions test/fixtures/example-ts-error-stack/app.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export default function() {
// placeholder comments
// placeholder comments
// placeholder comments
// placeholder comments
if (process.env.THROW_ERROR === 'true') {
throw new Error('throw error');
}
}
7 changes: 7 additions & 0 deletions test/fixtures/example-ts-error-stack/config/config.default.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

export default () => {
const config = {} as any;
config.keys = '123456';
return config;
};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions test/fixtures/example-ts-error-stack/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "example-ts-error-stack",
"egg": {
"typescript": true
}
}
16 changes: 16 additions & 0 deletions test/fixtures/example-ts-error-stack/test/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use strict';

// import * as assert from 'assert';

describe('test/index.test.ts', () => {
// placeholder comments
it('should throw error', async () => {
throw new Error('error');
});

// placeholder comments
it('should assert', async () => {
const obj = { key: '111' };
assert(obj.key === '222');
});
});
19 changes: 19 additions & 0 deletions test/fixtures/example-ts-error-stack/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"compilerOptions": {
"target": "es2017",
"module": "commonjs",
"strict": true,
"noImplicitAny": false,
"moduleResolution": "node",
"experimentalDecorators": true,
"emitDecoratorMetadata": true,
"pretty": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noFallthroughCasesInSwitch": true,
"skipLibCheck": true,
"skipDefaultLibCheck": true,
"inlineSourceMap": true,
"importHelpers": true
},
}
3 changes: 2 additions & 1 deletion test/fixtures/example-ts-pkg/node_modules/egg/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/fixtures/example-ts/node_modules/egg/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/fixtures/ts/node_modules/aliyun-egg/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 38 additions & 0 deletions test/ts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,44 @@ describe('test/ts.test.js', () => {
});
});

describe('error stacks', () => {
if (process.env.EGG_VERSION && process.env.EGG_VERSION === '1') {
console.log('skip egg@1');
return;
}

before(() => {
cwd = path.join(__dirname, './fixtures/example-ts-error-stack');
});

it('should correct error stack line number in starting app', () => {
mm(process.env, 'THROW_ERROR', 'true');
return coffee.fork(eggBin, [ 'dev' ], { cwd })
// .debug()
.expect('stderr', /Error: throw error/)
.expect('stderr', /at \w+ \(.+app\.ts:7:11\)/)
.end();
});

it('should correct error stack line number in testing app', () => {
return coffee.fork(eggBin, [ 'test' ], { cwd })
// .debug()
.expect('stdout', /error/)
.expect('stdout', /at Context\.it .+index\.test\.ts:8:11\)/)
.expect('stdout', /at Context\.it .+index\.test\.ts:14:5\)/)
.end();
});

it('should correct error stack line number in covering app', () => {
return coffee.fork(eggBin, [ 'test' ], { cwd })
// .debug()
.expect('stdout', /error/)
.expect('stdout', /at Context\.it .+index\.test\.ts:8:11\)/)
.expect('stdout', /at Context\.it .+index\.test\.ts:14:5\)/)
.end();
});
});

describe('egg.typescript = true', () => {
if (process.env.EGG_VERSION && process.env.EGG_VERSION === '1') {
console.log('skip egg@1');
Expand Down

0 comments on commit ca4f78f

Please sign in to comment.