7rulnik / source-map-js

Consume and generate source maps.
Other
95 stars 14 forks source link

`Error: original.line and original.column are not numbers` when a mapping entry only contains a single field. #18

Closed romainmenke closed 7 months ago

romainmenke commented 9 months ago

Hi!

I am a contributor to postcss and an issue was reported there that I think can only be solved here: https://github.com/postcss/postcss/issues/1914


Reproduction :

/* -*- Mode: js; js-indent-level: 2; -*- */

var util = require("./util");
var SourceMapGenerator = require('../lib/source-map-generator').SourceMapGenerator;
var SourceMapConsumer = require('../lib/source-map-consumer').SourceMapConsumer;

exports['test .fromSourceMap with only a single field in the last mapping entry'] = function (assert) {
  var map = SourceMapGenerator.fromSourceMap(
    new SourceMapConsumer(`{
    "version": 3,
    "file": "swagger-ui.css",
    "mappings": "AAAA,iBAWE,uBAFA,sBACA,kBAEA,oCAVA,eACA,gBAEA,iBAEA,kBAKA,CAEA,wBAEE,eACA,iB",
    "sources": [
        "webpack://swagger-ui/./src/style/_buttons.scss"
    ],
    "sourcesContent": [
        ".btn\\n{\\n  font-size: 14px;\\n  font-weight: bold;\\n\\n  padding: 5px 23px;\\n\\n  transition: all .3s;\\n\\n  border: 2px solid $btn-border-color;\\n  border-radius: 4px;\\n  background: transparent;\\n  box-shadow: 0 1px 2px rgba($btn-box-shadow-color,.1);\\n\\n  &.btn-sm\\n  {\\n    font-size: 12px;\\n    padding: 4px 23px;\\n  }\\n}\\n"
    ],
    "names": [],
    "sourceRoot": ""
}`));
  util.assertEqualMaps(assert, map.toJSON());
};

As far as I can tell it is allowed to have fewer fields in mapping segments. https://sourcemaps.info/spec.html#h.lmz475t4mvbx


I created the sourcemap by cloning https://github.com/swagger-api/swagger-ui and removing most of the CSS content to create a small output that was easier to read.

Any sourcemap produced by the setup in that project gives an error in source-map-js.

It is perfectly possible that the issue is with how the sourcemap is generated and not with how it is read.

romainmenke commented 9 months ago

Related issue in original repo : https://github.com/mozilla/source-map/issues/385

ai commented 9 months ago

Stacktrace of the error:

00:47:03  Error: [postcss] original.line and original.column are not numbers -- you probably meant to omit the original mapping entirely and only map the generated position. If so, pass null for the original mapping instead of an object with empty or null values.
00:47:03      at SourceMapGenerator.SourceMapGenerator_validateMapping [as _validateMapping] (/home/jenkins/workspace/Geiger/geiger-upgrade-deps/node_modules/.pnpm/source-map-js@1.0.2/node_modules/source-map-js/lib/source-map-generator.js:276:15)
00:47:03      at SourceMapGenerator.SourceMapGenerator_addMapping [as addMapping] (/home/jenkins/workspace/Geiger/geiger-upgrade-deps/node_modules/.pnpm/source-map-js@1.0.2/node_modules/source-map-js/lib/source-map-generator.js:110:12)
00:47:03      at /home/jenkins/workspace/Geiger/geiger-upgrade-deps/node_modules/.pnpm/source-map-js@1.0.2/node_modules/source-map-js/lib/source-map-generator.js:72:17
00:47:03      at BasicSourceMapConsumer.SourceMapConsumer_eachMapping [as eachMapping] (/home/jenkins/workspace/Geiger/geiger-upgrade-deps/node_modules/.pnpm/source-map-js@1.0.2/node_modules/source-map-js/lib/source-map-consumer.js:155:7)
00:47:03      at Function.SourceMapGenerator_fromSourceMap [as fromSourceMap] (/home/jenkins/workspace/Geiger/geiger-upgrade-deps/node_modules/.pnpm/source-map-js@1.0.2/node_modules/source-map-js/lib/source-map-generator.js:48:24)
00:47:03      at MapGenerator.generateMap (/home/jenkins/workspace/Geiger/geiger-upgrade-deps/node_modules/.pnpm/postcss@8.4.33/node_modules/postcss/lib/map-generator.js:101:37)
00:47:03      at MapGenerator.generate (/home/jenkins/workspace/Geiger/geiger-upgrade-deps/node_modules/.pnpm/postcss@8.4.33/node_modules/postcss/lib/map-generator.js:85:19)
00:47:03      at new NoWorkResult (/home/jenkins/workspace/Geiger/geiger-upgrade-deps/node_modules/.pnpm/postcss@8.4.33/node_modules/postcss/lib/no-work-result.js:33:46)
00:47:03      at Processor.process (/home/jenkins/workspace/Geiger/geiger-upgrade-deps/node_modules/.pnpm/postcss@8.4.33/node_modules/postcss/lib/processor.js:51:14)
00:47:03      at runPostcss (file:///home/jenkins/workspace/Geiger/geiger-upgrade-deps/node_modules/.pnpm/vite@4.5.0_@types+node@18.16.16_less@4.2.0/node_modules/vite/dist/node/chunks/dep-e4a495ce.js:287:6) {
00:47:03    code: 'PLUGIN_ERROR',
00:47:03    loc: { column: undefined, line: undefined },
7rulnik commented 7 months ago

Hey guys! We discussed it a bit in private with @ai and @alexander-akait

Alexander said that it's a known bug but he didn't have time to solve it because of private business in real life.

Andrey suggests skipping broken source maps. I am fine with this suggestion but the real problem is that nobody of us are familiar with source code and how it actually works. I will try to look deeper into the code but can't promise anything.

ai commented 7 months ago

Just not that I suggest skipping broken mapping (pos 1 > pos 2), not entire source map πŸ˜…

romainmenke commented 7 months ago

Thank you for the update @7rulnik

πŸ€” Should we then maybe consider moving away from source-map-js for PostCSS? If I understand it correctly source-map-js is a rescue project for source-map because it radically changed direction and became a rust project.

Finding a different package for source maps might be a better long term solution?

7rulnik commented 7 months ago

@romainmenke in my opinion β€” yes There are different implementations https://github.com/tc39/source-map/issues/47 But let's keep this issue related only to source-map-js

ai commented 7 months ago

@romainmenke can you find a good alternative with sync API?

ai commented 7 months ago

SourceMapConsumer is part of PostCSS public API. I can’t change the implementation.

7rulnik commented 7 months ago

https://github.com/7rulnik/source-map-js/releases/tag/v1.1.0 Huge thanks @ai! Closing this for now.

romainmenke commented 7 months ago

Thank you @ai, @7rulnik πŸ™‡