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

Set sourceMap file #97

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Set sourceMap file #97

wants to merge 1 commit into from

Conversation

nitely
Copy link

@nitely nitely commented Aug 15, 2020

Fixes #91

The lib vinyl-sourcemaps-apply will throw an error if the sourceMap file property is not set. They seem unwilling to remove the assertion that checks for the file property.

The changes seem to work with gulp-sourcemaps, but not with the gulp v4 built in sourcemaps, which doesn't work regardless.

The lib vinyl-sourcemaps-apply will throw an error if file is not set
@yocontra
Copy link
Member

@nitely Thanks for the PR - could you add a test for this so we can make sure sourcemaps are never broken again?

Also curious if the gulp built-in sourcemaps aren't working and you're already doing a test for this, if you do a failing test for that I can look into fixing that.

@nitely
Copy link
Author

nitely commented Aug 16, 2020

err, I checked the tests and they do test the sourcemaps are generated. I reverted the changes and ran the tests, and they pass (as I remembered).

The sourcemaps are generated, the file property is set. So, I went back to my project to try and reproduce the issue again. I found the only way to reproduce it is to pass the transpile option. Using babel at a later stage works just fine as a workaround, ex: .pipe(babel({presets: ['@babel/preset-env']})).

About the gulp built in sourcemaps, I found the dest function requires to pass { sourcemaps: true } as well, ex: .pipe(dest('output/', { sourcemaps: '.' }) to generate external sourcemaps, or .pipe(dest('output/', { sourcemaps: true })); to geenrate inlined sourcemaps. Otherwise they are not generated. It's in the docs.

@yocontra
Copy link
Member

@nitely Yeah that is expected - if you use just the gulp sourcemaps does this work or is the PR still needed?

@nitely
Copy link
Author

nitely commented Aug 18, 2020

The transpile option doesn't work. It throws the missing file property error when using either the gulp-sourcemaps, or the built-in sourcemaps.

gulp-babel on the other hand works fine with both gulp-sourcemaps and the built-in sourcemaps.

I can add tests for the transpile case, but I'll need to add babel as a devDependency, is that ok?

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.

coffeescript 2 with transpile + inlined source maps?
2 participants