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

null sourcesContent with 0.0.84 #1715

Open
jmm opened this issue Feb 9, 2015 · 10 comments
Open

null sourcesContent with 0.0.84 #1715

jmm opened this issue Feb 9, 2015 · 10 comments

Comments

@jmm
Copy link

jmm commented Feb 9, 2015

With traceur 0.0.84 the following results in null sourcesContent:

(new traceur.NodeCompiler({sourceMaps: 'inline', modules: 'commonjs'}))
  .compile("console.log('x');", '/a.js', '/a.js');

It appears to be related to the leading slash in outputName / the problem in #1695. This triggers it too:

(new traceur.NodeCompiler({sourceMaps: 'inline', modules: 'commonjs'}))
  .compile("console.log('x');", 'somedir/a.js', 'somedir/x.js');
@kornelski
Copy link

👍

It completely breaks sourcemaps in Webpack + source-map-loader.

@johnjbarton
Copy link
Contributor

Try removing the outputName argument. Does it make a difference?

@jmm
Copy link
Author

jmm commented Feb 27, 2015

Try removing the outputName argument. Does it make a difference?

It populates the sourcesContent, but populates "file":"<compileOutput>" instead of the value you want.

@johnjbarton
Copy link
Contributor

Modifying the sourcemap after it is created is relatively easy for these record properties.

@jmm
Copy link
Author

jmm commented Feb 27, 2015

Modifying the sourcemap after it is created is relatively easy for these record properties.

Your proposal is that in order to get sensible source map output when using traceur the user should have to take traceur's output, strip out sourceMappingURL comments, parse the source map, manipulate it, and add a sourceMappingURL comment back to the file? And all for what, so CLI users can get an auto-generated value that may not have anything to do with anything?

@johnjbarton
Copy link
Contributor

No, but maybe we should move the sourcemap inlining out of traceur. Just have utility API functions to deal with all of these issues. That would prevent traceur from blocking progress on the issues of sourcemaps that don't require parse info.

@jmm
Copy link
Author

jmm commented Feb 27, 2015

So then what would happen with the CLI? As far as I've been able to tell, trying to cater to some anticipated rigid correspondence between location of source / generated files on local disk and location of files on the web server serving the source map, for the sake of the CLI, is responsible for most or all of these weird path handling issues that have been mucking up the source mapping.

@johnjbarton
Copy link
Contributor

My goal, as you note, would be to keep the CLI working as it is unless there is a reason to change it. Breaking out the sourcemap functions in the API would allow API callers to be more flexible for cases they need and in the CLI we'll support the opinionated location of source/generated files.

Alternatively someone can propose pull requests that make the path handling less weird.

@jmm
Copy link
Author

jmm commented Feb 27, 2015

Ok, so we had sourceRoot being treated as a local path to alter sourceName values and that was discontinued, right? So now we're down to just outputName being used to alter sourceName values, right?

My goal, as you note, would be to keep the CLI working as it is unless there is a reason to change it.

The reason to change it is that it's the basis for making the API (and CLI) work in a screwy way for everyone else who doesn't want that behavior, who wants straightforward behavior for controlling the source map output. Why not make the baseline behavior predictable and straightforward and give CLI users a flag to opt-in to this opinionated behavior? Do a majority of CLI users even want that?

Breaking out the sourcemap functions in the API would allow API callers to be more flexible for cases they need and in the CLI we'll support the opinionated location of source/generated files.

That might be a good option to have anyway, but I'm not sure all usage of the API should require jumping through hoops just to work around an opinionated behavior that's assumed for the CLI. Another option could be to add a signature like this for the compile method:

// src is the same as currently
compile(src, sourceMap)

// sourceMap is an object that can include the following properties,
// which will be passed through to source map output unaltered and
// used for nothing else:
{
  // String that will be populated in the `sources` array
  sources: 'abc',
  // String that will be populated as the `file` property
  file: 'xyz',
  // String that will be populated as the `sourceRoot` property
  sourceRoot: '/'
}

That would be low overhead for API users because (although a bit more verbose) it's similar to the current signature and values that would've been passed as sourceName / outputName / sourceRoot would just be passed in the hash instead. This is just off the top of my head -- I'll consider it further.

@jmm
Copy link
Author

jmm commented Mar 2, 2015

Focusing for the moment on getting the API calls working in a sensible way, can't you just detect which requests are coming through the CLI and disable (omit) this opinionated input alteration for requests made via the API? Or failing that, at least do what I suggested on 2/16 and disable it when compile() is passed the content argument? You seem to think there's some logic to altering the paths when CLI users ask Traceur to read from and write to the file system, but what's the explanation for doing it when Traceur isn't asked to do either of those things?

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