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

[failed test] babel-transpiled module considered invalid #8

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

Conversation

dwickern
Copy link

The validator is giving errors from ember-cli, which uses babel to transpile es6->es5.

The test suite didn't seem to recognize the inline sourcemap in my first test so I pulled out the sourcemap and added a second test. The test fails with this error:

{ AssertionError [ERR_ASSERTION]: missing original column, mapping: {
  "generatedLine": 17,
  "generatedColumn": 23
}

Here's the sourcemap visualized

Looking at the decoded sourcemap, it seems that this entry on line 17 is the culprit:

{
  "fieldCount": 1,
  "col": 23,
  "src": 0,
  "srcLine": 0,
  "srcCol": 0,
  "name": 0
}

which corresponds to this line

  exports.default = App;

It seems to map line 17 col 23 in the output to line 0 col 0 in the source?

I don't know much about sourcemaps but it seems the validator is working properly here. I'm still opening this so I can link back to it from the ember project

@loganfsmyth
Copy link
Contributor

I think this is no longer an error due to #12 FYI.

@krisselden
Copy link

@loganfsmyth sourcemap should map the start of the expression not white space, safest is to mark the start/end of tokens, and should map all name mangling. The babel module is in the wrong for inserting whitespace inside of the mapping, that won't work with inbetween tooling like rollup, uglify etc.

@krisselden
Copy link

If you went from typescript to babel to rollup for example, rollup would map to whitespace which wouldn’t map to anything in typescript sourcemap. Babel needs stop adding formatting without adjusting mappings.

@loganfsmyth
Copy link
Contributor

loganfsmyth commented May 8, 2018

@krisselden My comment here was an honest attempt to let this user know that missing original column is no longer an error that this module throws when processing Babel code. I'm happy to have a conversation about further mapping issues, but I don't necessarily appreciate the demanding tone behind your comment.

sourcemap should map the start of the expression not white space

If this is referring to our current handling of expressions at the start of lines, I believe that was added specifically before otherwise debuggers failed to interpret our maps properly.

safest is to mark the start/end of tokens

Babel has no concept of original-file tokens in its AST, so there is no way for us to do this. We could certainly map individual generated tokens in the output, but I don't see how that would actually be better than the current behavior.

should map all name mangling

That we do as far as I'm aware.

The babel module is in the wrong for inserting whitespace inside of the mapping,

The source map spec in no way specifies how mappings are meant to be used, all we have is single range-to-range mappings. Many tools generate simple line-to-line mappings that have no concept of tokens to begin with, for instance, and those maps are still valuable even if they contain whitespace. Similarly, it's valid for an original identifier to map to a member expression, in which case it is entirely possible that there would be whitespace within the range of text being mapped.

Babel needs stop adding formatting without adjusting mappings.

There is no "adjusting mappings" to begin with and since you've provided no examples, there's no way for me to respond to this.

If you went from typescript to babel to rollup for example, rollup would map to whitespace which wouldn’t map to anything in typescript sourcemap.

I don't really follow what this is describing.

It feels like you're very frustrated with out mappings, but as far as I'm aware babel/babel#6008 is the only issue we have to track that. Perhaps it would be more productive to move this discussion there with specific examples.

@krisselden
Copy link

Opening a failing test on the validator that’s fails just because it fails to validate babel output as valid comes across as implying babel is right because well it’s babel. And that’s frustrating.

The issues with babel have to do with chaining sourcemaps not just going from babel straight to the debugger. And while a line by line source map may work ok with the debugger it does not work for example as the input map to a minifier.

That’s what the validator and visualizer are trying to show. That’s why the visualizer has a minify button to show the result of the minifier chaining its generated mappings back through your generated mappings to the original. Most of them will miss and be dropped in a line by line.

If you have source maps that pass this validation they will chain well with rollup or sorcery or as the input to uglify.

I will add more detail with examples tomorrow I just wanted to clarify.

Also, I’m very sorry I got so frustrated earlier. I should not have ranted at you.

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.

3 participants