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

Sourcemaps not being correctly mapped for preprocessed files. #460

Closed
techmatt101 opened this issue Nov 19, 2016 · 8 comments
Closed

Sourcemaps not being correctly mapped for preprocessed files. #460

techmatt101 opened this issue Nov 19, 2016 · 8 comments

Comments

@techmatt101
Copy link
Contributor

Given TS files after being preprocessed by gulp-concat or anything equivalent, the sourcesContent on sourcemaps generated result to null and sources listed are not the original src files. Below is my current setup.

Example Setup:

└── src/
   ├── apple.ts
   └── banana.ts
gulp.task('default', function () {
  return gulp.src('src/**/*.ts')
    .pipe(sourcemaps.init())
    .pipe(concat('all.ts'))
    .pipe(typescript())
    .pipe(sourcemaps.write())
    .pipe(gulp.dest('build'));
});

When opening build/all.js using source-map-visualization it fails to load. Error: Source 'all.ts' missing.

Expected source map output:

{"version":3,"sources":["apple.ts","banana.ts"],"names":[],"mappings":"AAAA;IAAA;IAAA,CAAA;IAAA,YAAA;AAAA,CAAA,IAAA;ACAA;IAAA;IAAA,CAAA;IAAA,aAAA;AAAA,CAAA,IAAA","file":"all.js","sourcesContent":["class Apple {}","class Banana {}"]}

Actual source map output:

{"version":3,"sources":["all.ts"],"names":[],"mappings":"AAAA;IAAA;IAAa,CAAC;IAAD,YAAC;AAAD,CAAb,AAAc,IAAA;AACd;IAAA;IAAc,CAAC;IAAD,aAAC;AAAD,CAAd,AAAe,IAAA","file":"all.js","sourcesContent":[null]}
@techmatt101
Copy link
Contributor Author

I've started debugging through the code looks like the issue is related to the file path being absolute and changing the file to a relative path seems to fix the issue.

Problem Code: output.ts@83

// sourceFile.fileNameOriginal = "C:\Users\Matt\Desktop\test\src\all.ts";
// sourceFile.gulp.base = "C:\Users\Matt\Desktop\test\src\";
generator.applySourceMap(consumer, sourceFile.fileNameOriginal, sourceFile.gulp.base);

Proposed Solution:

generator.applySourceMap(consumer, path.relative(sourceFile.gulp.base, sourceFile.fileNameOriginal), sourceFile.gulp.base);

Optional Solution:
sourceFile and sourceMapPath are optional parameters but involves reverting a code change from #6372612 commit (Fix issues with output paths).

generator.applySourceMap(consumer);

Any thoughts? Will this have any negative repercussion? All tests still pass after making the change.

@ivogabe
Copy link
Owner

ivogabe commented Nov 21, 2016

Thanks for reporting and investigating. I think that your proposed fix is the desired fix, though I would need to do some testing. The optional solution does not work well, the issue is that all paths (involved with source maps) are relative, but relative to different directories. Can you create a pull request with your proposed solution? Then I'll do some testing with it.

@techmatt101 techmatt101 changed the title Sourcemaps not being corretly mapped for preprocessed files. Sourcemaps not being correctly mapped for preprocessed files. Nov 30, 2016
@techmatt101
Copy link
Contributor Author

Just created a pull request for my "Optional/Second Solution". The reason for not going with my "Proposed Solution" is because I wasn't able to test using absolute paths as the expected result would be confined to my dev environment location ("C:\Users\Matt...."). Now thinking about it I could of used something like gulp-rename to work around the issue maybe? :/

I'm still not sure if it would be a problem, could you possibly have a look over it let me know what issue it causes so I can get a better understating. Might even be worth us adding a test round these problem scenarios.

@ivogabe
Copy link
Owner

ivogabe commented Nov 30, 2016

Thanks for the PR. The test case is a good addition to the test suite. I'm afraid that it might break some other scenario's, but I'll investigate that. I have my mind on other things now but I hope to do that this weekend. I'll try to construct a test case that demonstrates such a scenario.

Absolute paths should not be a problem since gulp-sourcemaps' documentation states that the paths in the sourcemap should be relative.

@giggio
Copy link

giggio commented Dec 26, 2016

I can't use gulp-typescript until this issue is solved. I am reverting back to gulp-exec or something like that.

ivogabe added a commit that referenced this issue Dec 27, 2016
fixed sourcemap sources being lost for files with existing mappings #460
@ivogabe
Copy link
Owner

ivogabe commented Dec 27, 2016

Fixed in #465

@ivogabe ivogabe closed this as completed Dec 27, 2016
@giggio
Copy link

giggio commented Jan 2, 2017

@ivogabe any idea of when this will make to a release?

@ivogabe
Copy link
Owner

ivogabe commented Jan 6, 2017

@giggio I want to go through some issues today and hope to release it today too.

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

3 participants