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

sourceMappingURL directive can fail if a bundled module specifies the directive #772

Open
davidmason opened this issue May 20, 2014 · 26 comments

Comments

@davidmason
Copy link

If a bundled module has a sourceMappingURL directive, browserify will leave it intact, which appears to be worthless or harmful towards getting source maps to work properly in browsers.

Ideally, when browserify is generating source maps it would detect sourceMappingURL directives in bundled modules and include the contents of the targeted source maps in the bundle's source map. At very least, the directives should be removed during bundling to prevent possible conflicts.

The same issue is present in webpack, so I will refer to the bug report there rather than duplicate it: webpack/webpack#273

The only difference between browserify and webpack with this issue is that browserify does not use the deprecated //@ form of the directive, so it will only see the issue of duplicate sourceMappingURL directives when the bundled module has a directive in the form //# sourceMappingURL=....

@thlorenz
Copy link
Collaborator

I don't think it's browserifys job to clean up source maps.

Instead it's job is to bundle your modules together. If you tell browserify to combine the source maps via --debug it will clean the ones it found and combined.
However in other cases it will not even look for them.

You should write a transform to remove source maps instead of making this part of browserify itself. This should be a fairly simple task, just use convert-source-map to find the source map via a regex and remove them from each file.

@davidmason
Copy link
Author

To clarify, the behaviour I'm seeing occurs only when using --debug to make browserify generate source maps.
Is browserify expected to find and combine source maps if it is just given --debug, or is there some configuration I'm missing that is preventing it from finding the source map of the dependency?

@thlorenz
Copy link
Collaborator

Ah that's different then, it is supposed to find them when given --debug.

So I'm not sure why in your case it doesn't find it.
Could you post a sample project to repro this issue somewhere on github?

@davidmason
Copy link
Author

I was getting the issue in https://github.com/davidmason/work-life

It needs a couple of tweaks to work in browserify:

The package that is not having its source map processed is https://github.com/meryn/performance-now (brought in via crtrdg-gameloop <- raf-stream <- raf <- performance-now)

@Bartvds
Copy link

Bartvds commented Jun 6, 2014

I think I have the same issue.

I compile TypeScript using an external compiler (grunt-ts, because the existing browserify TypeScript transforms/plugins are all borked).

So I have a directory with commonJS JavaScript modules, each with a //# sourceMappingURL directive and a map file pointing to the TypeScript sources.

Then I use browserify with debug: true on the main file, but it ignores the existing directives and the bundle's sourcemap points to the intermediate commonJS JavaScript instead of the original TypeScript.

When I look into the bundled JavaScript I see the original //# sourceMappingURL directives are not removed.

I tried it with plain browserify (and a base64 decoder to inspect the inline sourcemap) and also with exorcist to output a separate file.

Note I'm on working on windows, if that matters for anything.

@Bartvds
Copy link

Bartvds commented Jun 6, 2014

I created a minimal demo for this: https://github.com/Bartvds/demo-typescript-browserify

It uses grunt and grunt-ts to compile two TypeScript files to commonJS modules, and then a simple inline task to run browserify+exorcist to bundle them.

For your review I checked-in the JS code:

The intermediate JavaScript:

https://github.com/Bartvds/demo-typescript-browserify/tree/master/tmp

And the bundle file + exorcised sourcemap:

https://github.com/Bartvds/demo-typescript-browserify/tree/master/dist

As you see the bundle file still has the original directives, and the sourcemap points to the intermediate files instead of TypeScript sources.

@terinjokes
Copy link
Contributor

browser-pack is the module that is responsible for inserting the source maps generated by {debug: true}, and also where existing source maps are removed. I'm sure a patch to merge the two source maps, so you get the original source, would be appreciated.

mprobst added a commit to mprobst/node-source-map-support that referenced this issue May 28, 2015
According to the spec (https://goo.gl/UQJKOW) the source map comment should be
at the very end of the file. Browser's adhere to that, i.e. use the last source
map in the file.

Browserify's --debug adds a source map at the bottom of the compiled file, but
currently fails to combine that with the source map referenced in script.js.
This is a bug in Browserify, see issue
browserify/browserify#772.

Our test case thus only worked by accident, for the user in the browser only
"script.js" shows up, not "script.coffee" as it should.

So for the time being, we should just remove the --debug flag from the build and
re-enable once browserify fixes that issue.
mprobst added a commit to mprobst/node-source-map-support that referenced this issue May 28, 2015
According to the spec (https://goo.gl/UQJKOW) the source map comment should be
at the very end of the file. Browser's adhere to that, i.e. use the last source
map in the file.

Browserify's --debug adds a source map at the bottom of the compiled file, but
currently fails to combine that with the source map referenced in script.js.
This is a bug in Browserify, see issue
browserify/browserify#772.

Our test case thus only worked by accident, for the user in the browser only
"script.js" shows up, not "script.coffee" as it should.

So for the time being, we should just remove the --debug flag from the build and
re-enable once browserify fixes that issue.
@OvidiuGuta
Copy link

Hi guys. Are there any news on this?

Since libraries like Angular2 use TypeScript as a first class citizen, I think we will see a lot more modules on npm developed in Typescript.

Personally, I like having the possibility to step into the libraries I'm using when debugging, so I think this feature is really useful.

@quantizor
Copy link

I'm also running into this issue. I have a module dependency that is pre-compiled with its own sourcemap and when I run browserify with --debug on my code, it does not bring in the inline sourcemap from the module dependence, so it's like it isn't even there.

@0815fox
Copy link

0815fox commented Mar 9, 2016

+1 same issue here.

@bradleat
Copy link

+1 for this issue

@DirtyHairy
Copy link

+1

1 similar comment
@sassanh
Copy link

sassanh commented Jul 8, 2016

+1

@sassanh
Copy link

sassanh commented Jul 8, 2016

I guess even if we don't combine sourcemaps yet, we should eliminate sourceMappingURL directive from libraries when building the bundle with --debug. This way we won't see annoying warnings in console when building a library that has sourceMappingURL directives inside.

@levibotelho
Copy link

Is the correct behaviour that preexisting sourcemaps for files that are being bundled by browserify should be integrated into the final bundle sourcemap -- that is to say that sourcemaps should reverse all compilation steps and not just the browserify step?

If this is so what needs to be done to fix the issue? I'd be happy to pitch in with a PR if needed. Can someone familiar with browserify provide any additional details?

@XanderEmu
Copy link

+1

@bruce965
Copy link

This question from Stack Overflow contains some workarounds that might be useful to other developers.

http://stackoverflow.com/q/32486196/1135019

@jbeard4
Copy link

jbeard4 commented May 2, 2017

As described in the stack overflow article, I was able to use the sorcery package to fix sourcemaps generated by browserify and work around this issue.

@dy
Copy link

dy commented Mar 16, 2019

That was my expectation from browserify too - to consume sourcemaps provided by input files, but turns out it just ignores them. swcify sourcemaps support is blocked by that.
@goto-bus-stop what do you think, should there be a PR for that?

@goto-bus-stop
Copy link
Member

browser-pack should merge source maps correctly afaik. transforms are individually responsible for reading input source maps and outputting merged ones. i'm not sure what specifically is going wrong for people in this issue, but with a repro i could take a look at it

@jeremija
Copy link

jeremija commented Mar 25, 2019

Hi @goto-bus-stop, I'm afraid I don't have a public example at the moment, but I'll do my best to describe my experience. I have a TypeScript library and a main project which references the library via TypeScript project references, so whenever I run tsc -b in the main project, both the main project the library are built. I believe tsify does not know about project references yet since this functionality is fairly new, and some of TypeScript's own tooling (like tsserver) does not fully support it yet, so I have to build the library manually via tsc, or the main project (together with the library) via tsc -b. The library is symlinked into node_modules so I can import from the main project.

Since the library is built via tsc, and not through tsify (because tsify only builds the "main" project, and uses whatever is available in node_modules/library), the source maps are not generated in the same manner. The only available source maps for my library are the ones built by TypeScript. For example, the sourceMappingURL for node_modules/library/lib/myfile.js looks like:

//# sourceMappingURL=myfile.js.map`

The browser-pack module uses the combine-source-map module, which then uses the convert-source-map module. Looks like convert-source-map only looks for embedded source maps and ignores source map references like the one above. Here is the relevant RegExp from convert-source-map:

var commentRx = /^\s*\/(?:\/|\*)[@#]\s+sourceMappingURL=data:(?:application|text)\/json;(?:charset[:=]\S+;)?base64,(.*)$/mg;

My workaround as of yesterday is to use the sourceify library @dy referenced, but I had to patch it to read the sourcemap(s) in the sourcesContent property. I've described the work in detail in the pull request. I also had to use a global transform (-g [ sourceify ]). Before that pull request is merged, I invite anybody interested to test it:

npm install https://github.com/jeremija/sourceify#sources-content

Having said that, it would be very nice if all of this could just work out of the box :)

@jeremija
Copy link

jeremija commented Mar 26, 2019

I just added a sample repo here: https://github.com/jeremija/lerna-ts-example. Instructions provided in README.md.

EDIT I just realized that there is an easier way to do this in TypeScript, without the need for sourceify. The sourceMap property needs to be removed from tsconfig.json, and both inlineSourceMap and inlineSources need to be set to true.

Still, sourceify is useful in cases of 3rd-party libraries which might have source map file references instead of inlined sources.

@dy
Copy link

dy commented Mar 26, 2019

@goto-bus-stop nvm, swc generates wrong sourcemaps, swc-project/swc#349, so I was thinking browserify does not support them. It does.

@mirabilos
Copy link

Apparently it works only if you inline the source maps into browserify’s input files. But then it works well.

@reverofevil
Copy link

@mirabilos

Did you know? This bundler is barely ever used outside of legacy projects since 2016.

@mirabilos
Copy link

mirabilos commented Feb 2, 2023 via email

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

No branches or pull requests