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

Sources relative paths rewritten incorrectly #538

Closed
Tiberriver256 opened this issue Sep 3, 2017 · 9 comments
Closed

Sources relative paths rewritten incorrectly #538

Tiberriver256 opened this issue Sep 3, 2017 · 9 comments

Comments

@Tiberriver256
Copy link

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
Copy link
Owner

ivogabe commented Sep 20, 2017

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
Copy link
Author

Tiberriver256 commented Sep 20, 2017

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
Copy link
Owner

ivogabe commented Sep 20, 2017

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
Copy link
Author

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

@ivogabe
Copy link
Owner

ivogabe commented Dec 27, 2017

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:

  • If you don't provide the outDir option to TypeScript, the sourceRoot option of gulp-sourcemaps should be the relative path from the gulp.dest path to the source directory (from gulp.src)
  • If you set the outDir option to the same value as the directory in gulp.dest, you should set the sourceRoot to ./.
  • If you set the outDir option to a different value, there is no easy rule to configure gulp-sourcemaps. I'd advise to change the value of outDir if possible.

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

@Tiberriver256
Copy link
Author

Awesome, thanks ivogabe!

@ivogabe
Copy link
Owner

ivogabe commented Dec 27, 2017

@Tiberriver256 Have you tried in on your own project?

@Tiberriver256
Copy link
Author

Tiberriver256 commented Dec 28, 2017

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

@Tiberriver256
Copy link
Author

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"));
});

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

No branches or pull requests

2 participants