ivogabe / gulp-typescript

A TypeScript compiler for gulp with incremental compilation support.
MIT License
839 stars 129 forks source link

Sources relative paths rewritten incorrectly #538

Closed Tiberriver256 closed 6 years ago

Tiberriver256 commented 7 years ago

Expected behavior: Sources file paths in compiled .js.map files should be have correct relative paths for files in subfolders.

Directory structure:

.
+-- src
|   +-- main.ts
|   +-- subfolder
|   |   +-- MyFile.ts
+-- dist
|   +-- main.js
|   +-- main.js.map
|   +-- subfolder
|   |   +-- MyFile.js
|   |   +-- MyFile.js.map

MyFile.js.map should begin with

{"version":3,"sources":["../../src/subfolder/MyFile.ts"] ...

Actual behavior:

MyFile.js.map and begins with

{"version":3,"sources":["../src/subfolder/MyFile.ts"] ...

Your gulpfile:

const gulp = require('gulp');
const ts = require('gulp-typescript');
var sourcemaps = require('gulp-sourcemaps');

// pull in the project TypeScript config
const tsProject = ts.createProject('tsconfig.json');

gulp.task('scripts', () => {
  var tsResult = tsProject.src()
    .pipe(sourcemaps.init()) // This means sourcemaps will be generated
    .pipe(tsProject());

  return tsResult.js
    .pipe(
      sourcemaps.write('.')
    )
    .pipe(gulp.dest(tsProject.options.outDir));
});

tsconfig.json

Include your tsconfig, if related to this issue.

{
  "compilerOptions": {
    "target": "es6",
    "module": "commonjs",
    "outDir": "dist"
  },
  "compileOnSave": true,
  "include": [
    "src/**/*.ts"
  ],
  "exclude": [
    "node_modules"
  ]
}

Running the debugger it looks like the output from the typescript compiler actually has the correct relative path but gets rewritten when it hits this line (46) of release/output.js

map.sources = map.sources.map(relativeToOutput);
ivogabe commented 7 years ago

When looking at the docs of gulp-sourcemaps this seems to be correct. Quote:

Important: Make sure the paths in the generated source map (file and sources) are relative to file.base

As an example, consider /src/subfolder/MyFile.ts, which is compiled to /dist/subfolder/MyFile.ts. It's base path is /dist, and the paths in its sourcemap should be relative to this directory.

So I think that this behavior is correct, but not always desired. I'd suggest you to take a look at the docs of gulp-sourcemaps, I think that setting the sourceRoot option of sourcemaps.write to src or ../src would help, but I'm not that familiar with their API.

Tiberriver256 commented 7 years ago

Thanks for the reply ivogabe.

They do actually have documentation for adding relative paths that looks like this:

sourcemaps.write('.', {includeContent: false, sourceRoot: '../src'})

If you set the sourceRoot path it will override whatever sourceRoot is set in the compiler and replace it with this static path. This is the code from gulp-sourcemaps (you can tell if sourceRoot is set in the options it sets the root the same for all files regardless of how they are nested):

  function setSourceRoot(file) {
    var debug = rootDebug.spawn('setSourceRoot');

    var sourceMap = file.sourceMap;
    if (typeof options.sourceRoot === 'function') {
      debug(function() { return 'is function'; });
      sourceMap.sourceRoot = options.sourceRoot(file);
    } else {
      debug(function() { return 'from options'; });
      sourceMap.sourceRoot = options.sourceRoot;
    }
    if (sourceMap.sourceRoot === null) {
      debug(function() { return 'undefined'; });
      sourceMap.sourceRoot = undefined;
    }
  }

My reason for submitting this bug though is that if you leave off the sourceRoot option it uses the sourceRoot set by the compiler. The typescript compiler actually returns a proper relative path but this module seems to overwrite it in the relativeToOutput function I modified in output.ts.

I was thinking gulp-typescript could either A) not modify the sources attribute and assume the compiler output would be correct or B) re-write the path with proper relative sources path.

It would make gulp-typescript more reliable for other modules that may use the sources path and expect it to be a proper relative path.

ivogabe commented 7 years ago

Thanks for the clarification, I agree that we should do something about it. Option A wouldn't match the description of gulp-sourcemaps, but option B sounds like a good idea. This can break some users so this will be something for the next major release.

Tiberriver256 commented 7 years ago

Cool. I believe my PR should be a good start when the next release rolls around. Thanks for this project, super useful.

ivogabe commented 6 years ago

I've taken another look at it, and compared the output of gulp-typescript with gulp-sourcemaps' identityMap. The paths are generated the same way, so I believe that the paths of gulp-typescript are correct. The sourceRoot property is not present in the source maps generated by identityMap, which is also the current behavior of gulp-typescript.

I found that setting a relative path as sourceRoot will actually work. gulp-sourcemaps will automatically modify the sourceRoot for files in subdirectories (i.e. add ../ for each subdirectory). However, the correct value for sourceRoot depends on your TypeScript configuration. See ivogabe/gulp-typescript-sourcemaps-demo for some examples. The basic rule that I found:

I've updated the readme and its examples with this information.

Tiberriver256 commented 6 years ago

Awesome, thanks ivogabe!

ivogabe commented 6 years ago

@Tiberriver256 Have you tried in on your own project?

Tiberriver256 commented 6 years ago

Sorry not yet. I'll give it a shot tonight with the fake project structure given at the start of this issue..

Tiberriver256 commented 6 years ago

That worked. As you suggested removing outDir from tsconfig and just using gulp.dest seems to have done the trick.

Final adjusted working files (using the same structure):

tsconfig.json

{
  "compilerOptions": {
    "target": "es6",
    "module": "commonjs",
    "sourceMap": true
  },
  "compileOnSave": true,
  "include": [
    "src/**/*.ts"
  ],
  "exclude": [
    "node_modules"
  ]
}

gulpfile.js

const gulp = require('gulp');
const ts = require('gulp-typescript');
var sourcemaps = require('gulp-sourcemaps');

// pull in the project TypeScript config
const tsProject = ts.createProject('tsconfig.json');

gulp.task('scripts', () => {
  var tsResult = tsProject.src()
    .pipe(sourcemaps.init()) // This means sourcemaps will be generated
    .pipe(tsProject());

  return tsResult.js
    .pipe(
      sourcemaps.write({sourceRoot: '../src'})
    )
    .pipe(gulp.dest("./dist"));
});