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

Generate sourcemap #5

Open
mamciek opened this issue Oct 12, 2015 · 12 comments
Open

Generate sourcemap #5

mamciek opened this issue Oct 12, 2015 · 12 comments
Assignees
Labels

Comments

@mamciek
Copy link

mamciek commented Oct 12, 2015

Is it possible to pass option to jspm to tell it that it should generate sourcemap for bundle?

@buddhike
Copy link
Owner

@mamciek This options is already there but is not in the docs yet because we had a problem with it. I just fixed the underlaying issue. Will update the docs after running a few end-to-end tests. In the meantime give latest bits a whirl and see how that works out for you.

To enable it for all bundles use bundleOptions property:

jspm({
  bundleOptions: {
    sourceMaps: true
  }
});

You can also specify it as an option to a specific bundle:

jspm({
  bundles: [
    { src: 'a', dst: 'a-bundle.js', options: { sourceMaps: true } }
  ]
});

HTH

@buddhike buddhike added the bug label Oct 15, 2015
@buddhike buddhike self-assigned this Oct 15, 2015
@mamciek
Copy link
Author

mamciek commented Oct 19, 2015

it works but it does not add sourceMappingUrl at the end of file like so:
//# sourceMappingURL=app.bundle.js.map

@unional
Copy link

unional commented Nov 12, 2015

I try to bundle ts files and the sourcemap generated is basically empty: {"version":3,"sources":[],"names":[],"mappings":"","file":"undefined"}.

This happens when I remove external dependencies: src: 'src/index.ts - lodash'

What is the right way to generate the sourcemap needed?

@buddhike
Copy link
Owner

@unional another user also reported a problem related to building .ts files. I just opened a new issue to track this #10

@OliverJAsh
Copy link

I am also seeing what @mamciek is seeing—there is no sourceMappingUrl appended? Is this a bug in the gulp lib or systemjs-builder itself?

Also, is this compatible with https://github.com/floridoo/gulp-sourcemaps? When I use jspm+gulp-sourcemaps+gulp-rev, something goes badly wrong:

gulp.task('build-app', () => (
    jspmBuild({
        bundleSfx: true,
        bundleOptions: { minify: true, sourceMaps: true, sourceMapContents: true },
        bundles: [ { src: 'main', dst: 'main-bundle.js' } ]
    })
        .pipe(sourcemaps.init())
        .pipe(rev())
        .pipe(sourcemaps.write('.'))
        .pipe(gulp.dest('./public/js'))
        .pipe(rev.manifest())
        .pipe(gulp.dest('./public/js'))
));

Output:

public
└── js
    ├── config-85de1bd90f.js
    ├── config-85de1bd90f.js.map
    ├── main-bundle-01e14cc95b.js.map
    ├── main-bundle-01e14cc95b.js.map.map
    ├── main-bundle-16f1bdbc8d.js
    ├── main-bundle-16f1bdbc8d.js.map
    └── rev-manifest.json

@OliverJAsh
Copy link

systemjs-builder only adds the sourceMappingUrl comment when saving to disk, so we need to add it ourselves: systemjs/builder#406 (comment)

@buddhike
Copy link
Owner

@OliverJAsh Thanks for chasing this up. I've pushed a version which appends the sourceMappingUrl comment. However, it seems to be a bit tricky to make it work for non sfx bundles. When systemjs downloads a bundled package, it wraps it around a call to System.register... so the comment appended to the file ends up in that :-)
Wonder if this is a bug in systemjs. I didn't have much time to look at the details. Will dig it a bit deeper soon.

@buddhike
Copy link
Owner

@mamciek Could you please check if 0.0.13 works for you? Thanks!

@OliverJAsh
Copy link

I've just been looking into gulp-sourcemap. For support with that you need to use https://github.com/floridoo/vinyl-sourcemaps-apply, which handles adding the comment for you. I highly recommend this so the source map can be piped to other tasks, renamed, etc.

@buddhike
Copy link
Owner

This actually makes perfect sense. Very cool idea. Does it also mean, we can perhaps remove minify support as well? Because you can use gulp plugin for minify.

@OliverJAsh
Copy link

Perhaps, but maybe just keep it in there for ultimate flexibility?

On Wed, 25 Nov 2015 at 10:36 Buddhike de Silva notifications@github.com
wrote:

This actually makes perfect sense. Very cool idea. Does it also mean, we
can perhaps remove minify support as well? Because you can use gulp plugin
for minify.


Reply to this email directly or view it on GitHub
#5 (comment)
.

@buddhike
Copy link
Owner

Sure, agreed!
On Wed, 25 Nov 2015 at 9:44 PM, Oliver Joseph Ash notifications@github.com
wrote:

Perhaps, but maybe just keep it in there for ultimate flexibility?

On Wed, 25 Nov 2015 at 10:36 Buddhike de Silva notifications@github.com
wrote:

This actually makes perfect sense. Very cool idea. Does it also mean, we
can perhaps remove minify support as well? Because you can use gulp
plugin
for minify.


Reply to this email directly or view it on GitHub
<
#5 (comment)

.


Reply to this email directly or view it on GitHub
#5 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants