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

Broken applied result #242

Closed
ai opened this issue Jul 10, 2016 · 4 comments
Closed

Broken applied result #242

ai opened this issue Jul 10, 2016 · 4 comments

Comments

@ai
Copy link

ai commented Jul 10, 2016

I uses your awesome source-map in PostCSS to generate new source map and apply old to new one (merge them).

I have a report, that result after apply is broken. But I have no idea what I do wrong. Maybe I don’t understand something. Or maybe we really have issue in source-map here.

  1. PostCSS source map
  2. Sass source map
  3. Sass + PostCSS

Look at #board a selector, after apply it miss a mapping.

In PostCSS I apply map in common way:

let prevMap = new mozilla.SourceMapConsumer(prev.text);
finalMap.applySourceMap(prevMap, from, relativePath(root));

Origin issue was created by @winamp and he provided good playground for this issue.

@fitzgen tell me if you need more information

@lydell
Copy link
Contributor

lydell commented Jul 11, 2016

I have managed to reduce this down to a minimal test case:

var sourceMap = require('source-map')

var map = {
  "version": 3,
  "file": "sass.css",
  "sources": [
    "sass.scss"
  ],
  "mappings": "AAAA,AAAA,CAAC,CAAC;EACA,CAAC,EAAE,CAAE,GAIN;EALD,AAEE,CAFD,CAEC,CAAC,CAAC;IACA,CAAC,EAAE,CAAE,GACN",
  "names": [],
  "sourceRoot": ""
}

var consumer = new sourceMap.SourceMapConsumer(map)

console.log(consumer.originalPositionFor({line: 3, column: 2}))
// Actual:
//   { source: 'sass.scss', line: 1, column: 0, name: null }
// Expected:
//   { source: 'sass.scss', line: 3, column: 2, name: null }

This visualization of map in the code above shows that the character at line 3 column 2 maps back to line 3 column 2.

@lydell
Copy link
Contributor

lydell commented Jul 13, 2016

I added this at the end of the code snippet in my last comment:

consumer.eachMapping(function (m) {
  console.log(m)
})

Here is the output: (click to view)

{ source: 'sass.scss', line: 1, column: 0, name: null }
{ source: 'sass.scss',
  generatedLine: 1,
  generatedColumn: 0,
  originalLine: 1,
  originalColumn: 0,
  name: null }
{ source: 'sass.scss',
  generatedLine: 1,
  generatedColumn: 0,
  originalLine: 1,
  originalColumn: 0,
  name: null }
{ source: 'sass.scss',
  generatedLine: 1,
  generatedColumn: 1,
  originalLine: 1,
  originalColumn: 1,
  name: null }
{ source: 'sass.scss',
  generatedLine: 1,
  generatedColumn: 2,
  originalLine: 1,
  originalColumn: 2,
  name: null }
{ source: 'sass.scss',
  generatedLine: 2,
  generatedColumn: 2,
  originalLine: 2,
  originalColumn: 2,
  name: null }
{ source: 'sass.scss',
  generatedLine: 2,
  generatedColumn: 3,
  originalLine: 2,
  originalColumn: 3,
  name: null }
{ source: 'sass.scss',
  generatedLine: 2,
  generatedColumn: 5,
  originalLine: 2,
  originalColumn: 5,
  name: null }
{ source: 'sass.scss',
  generatedLine: 2,
  generatedColumn: 6,
  originalLine: 2,
  originalColumn: 7,
  name: null }
{ source: 'sass.scss',
  generatedLine: 2,
  generatedColumn: 9,
  originalLine: 6,
  originalColumn: 1,
  name: null }
{ source: 'sass.scss',
  generatedLine: 3,
  generatedColumn: 2,
  originalLine: 1,
  originalColumn: 0,
  name: null }
{ source: 'sass.scss',
  generatedLine: 3,
  generatedColumn: 2,
  originalLine: 3,
  originalColumn: 2,
  name: null }
{ source: 'sass.scss',
  generatedLine: 3,
  generatedColumn: 3,
  originalLine: 1,
  originalColumn: 1,
  name: null }
{ source: 'sass.scss',
  generatedLine: 3,
  generatedColumn: 4,
  originalLine: 3,
  originalColumn: 2,
  name: null }
{ source: 'sass.scss',
  generatedLine: 3,
  generatedColumn: 5,
  originalLine: 3,
  originalColumn: 3,
  name: null }
{ source: 'sass.scss',
  generatedLine: 3,
  generatedColumn: 6,
  originalLine: 3,
  originalColumn: 4,
  name: null }
{ source: 'sass.scss',
  generatedLine: 4,
  generatedColumn: 4,
  originalLine: 4,
  originalColumn: 4,
  name: null }
{ source: 'sass.scss',
  generatedLine: 4,
  generatedColumn: 5,
  originalLine: 4,
  originalColumn: 5,
  name: null }
{ source: 'sass.scss',
  generatedLine: 4,
  generatedColumn: 7,
  originalLine: 4,
  originalColumn: 7,
  name: null }
{ source: 'sass.scss',
  generatedLine: 4,
  generatedColumn: 8,
  originalLine: 4,
  originalColumn: 9,
  name: null }
{ source: 'sass.scss',
  generatedLine: 4,
  generatedColumn: 11,
  originalLine: 5,
  originalColumn: 3,
  name: null }

As you can see, there is a duplicate mapping:

{ source: 'sass.scss',
  generatedLine: 3,
  generatedColumn: 2,
  originalLine: 1,
  originalColumn: 0,
  name: null }
{ source: 'sass.scss',
  generatedLine: 3,
  generatedColumn: 2,
  originalLine: 3,
  originalColumn: 2,
  name: null }
  • The visualizer shows the second mapping, and ignores the first.
  • .applySourceMap() seems to do it the other way around: Use the first mapping and ignore the second.

Conclusions:

  • The visualizer could possibly benefit from somehow showing duplicate mappings.
  • This library might want to handle duplicate mappings some way – but how?
  • What does the spec say about duplicate mappings?
  • Sass shouldn’t output duplicate mappings in the first place. I’ve opened Duplicate source mapping sass/libsass#2122 about that.

@tromey
Copy link
Contributor

tromey commented Sep 22, 2017

This library might want to handle duplicate mappings some way – but how?

I've run into this as well, but I tend to think that the library should work on a GIGO principle. That is, it can pick either mapping and that's ok -- there's no way to guess which one is more correct.

It would be good, though, to ensure that this library doesn't generate duplicate mappings.

What does the spec say about duplicate mappings?

Nothing, unfortunately.

@jkrems
Copy link
Collaborator

jkrems commented Apr 18, 2021

Duplicate of #61

@jkrems jkrems closed this as completed Apr 18, 2021
@jkrems jkrems marked this as a duplicate of #61 Apr 18, 2021
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

4 participants