systemjs / builder

SystemJS build tool
MIT License
465 stars 122 forks source link

TypeError: ...: aStr.charAt is not a function when bundling #771

Open peteruithoven opened 7 years ago

peteruithoven commented 7 years ago

We're currently still using JSPM 0.17.0-beta.32 (SystemJS 0.19.41 and systemjs-builder 0.15.34) because the hot-reloader isn't compatible yet with SystemJS 0.20. Also

We have a big code code base and to decrease the initial load & transpile time I'm trying to bundle all the dependencies, doing something like:

jspm bundle -id 'src/js/index.js - [src/**/*]' bundle-deps.js

But this fails, to simply the issue I also tried to bundle the whole app, but this results in a similar error:

$ jspm bundle src/js/index.js 
     Building the bundle tree for src/js/index.js...

err  TypeError: src/js/utils/trace.js: aStr.charAt is not a function
    at SourceMapConsumer_parseMappings [as _parseMappings] (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/babel-core/node_modules/source-map/lib/source-map-consumer.js:453:16)
    at SourceMapConsumer.get (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/babel-core/node_modules/source-map/lib/source-map-consumer.js:68:12)
    at SourceMapConsumer_eachMapping [as eachMapping] (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/babel-core/node_modules/source-map/lib/source-map-consumer.js:140:22)
    at /home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/babel-core/lib/transformation/file/index.js:465:26
    at File.mergeSourceMap (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/babel-core/lib/transformation/file/index.js:490:8)
    at File.generate (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/babel-core/lib/transformation/file/index.js:729:25)
    at File.transform (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/babel-core/lib/transformation/file/index.js:564:17)
    at /home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/babel-core/lib/transformation/pipeline.js:50:19
    at File.wrap (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/babel-core/lib/transformation/file/index.js:574:16)
    at Pipeline.transform (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/babel-core/lib/transformation/pipeline.js:47:17)
    at Object.exports.compile (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/systemjs-builder/compilers/esm.js:116:22)
    at /home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/systemjs-builder/lib/compile.js:117:43
    at tryCatcher (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/bluebird/js/release/promise.js:510:31)
    at Promise._settlePromise (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/bluebird/js/release/promise.js:567:18)
    at Promise._settlePromiseCtx (/home/peteruithoven/Projects/Doodle3D/dev/Doodle3D-App/node_modules/bluebird/js/release/promise.js:604:10)

src/js/utils/trace.js is code in our project, but it doesn't include a charAt method call. the file, one item back in the stack trace does (/node_modules/babel-core/node_modules/source-map/lib/source-map-consumer.js).

This made me think it had something to do with sourcemaps, so I tried the same with --skip-source-maps and --source-map-contents but this gives the same result.

It seems to happen on the following line: https://github.com/mozilla/source-map/blob/master/lib/source-map-consumer.js#L439 By turning that into the following bundling seems to be fixed. But that's not really a solution and I'd like to know the cause.

      if (!aStr.charAt) index++;
      if (aStr.charAt(index) === ';') {

The source-map package seems up to date with version 0.5.6. I haven't found any issues that include charAt, but this one might be relevant: https://github.com/mozilla/source-map/issues/250 There is a pull request with a fix, but it hasn't been merged yet.

guybedford commented 7 years ago

Awesome thanks for tracking this down so nicely.

Does it perhaps work to use aStr[index] instead of charAt?

guybedford commented 7 years ago

Also maybe we just need to do a string cast or similar at the start of the function?

peteruithoven commented 7 years ago

But... the weird thing is that when I add logs like the following, it's not logged just before that error, and when it's logged it looks fine.

console.log('typeof aStr: ', typeof aStr);
console.log('aStr.charAt: ', aStr.charAt);

Also, console.logs in places that should be executed after mappings are parsed is logged before I get the error: https://github.com/babel/babel/blob/v6.21.0/packages/babel-core/src/transformation/file/index.js#L405

So even though the stacktrace points to https://github.com/mozilla/source-map/blob/master/lib/source-map-consumer.js#L439 and that line contains a aStr.charAt I'm not convinced that is where the issue is.

peteruithoven commented 7 years ago

I even added the following before https://github.com/mozilla/source-map/blob/master/lib/source-map-consumer.js#L439 but it never gets logged.

if (!aStr.charAt) {
  console.log('charAt != function');
}
peteruithoven commented 7 years ago

Looks like it was logging quite some things in between charAt != function and the actual error. I'm now only logging something when !aStr.charAt. Looks like in two cases aStr contains an Array. This is the second occurrence:

aStr.charAt:  undefined
typeof aStr:  object
instanceof Array:  true
aSourceRoot:  null
aStr:  [ [ [ 0, 0, 0, 0 ],
    [ 16, 0, 0, 16 ],
    [ 26, 0, 0, 26 ],
    [ 29, 0, 0, 16 ],
    [ 31, 0, 0, 31 ],
    [ 34, 0, 0, 16 ],
    [ 36, 0, 0, 36 ],
    [ 41, 0, 0, 16 ],
    [ 43, 0, 0, 43 ] ],
  [ [ 0, 0, 1, 2 ],
    [ 6, 0, 1, 6 ],
    [ 13, 0, 1, 13 ],
    [ 16, 0, 1, 2 ],
    [ 18, 0, 1, 18 ] ],
  [ [ 0, 0, 2, 4 ],
    [ 11, 0, 2, 11 ],
    [ 25, 0, 2, 4 ],
    [ 26, 0, 2, 26 ],
    [ 29, 0, 2, 4 ],
    [ 31, 0, 2, 31 ],
    [ 34, 0, 2, 4 ],
    [ 36, 0, 2, 36 ] ],
  [ [ 0, 0, 3, 6 ], [ 13, 0, 3, 13 ], [ 18, 0, 2, 36 ] ],
  [ [ 0, 0, 4, 6 ], [ 18, 0, 4, 18 ], [ 22, 0, 2, 36 ] ],
  [ [ 0, 0, 5, 6 ], [ 20, 0, 5, 20 ], [ 24, 0, 2, 36 ] ],
  [ [ 0, 0, 6, 6 ], [ 16, 0, 6, 16 ] ],
  [ [ 0, 0, 2, 36 ], [ 5, 0, 2, 4 ] ],
  [ [ 0, 0, 8, 3 ], [ 3, 0, 1, 2 ], [ 9, 0, 8, 9 ] ],
  [ [ 0, 0, 9, 4 ],
    [ 8, 0, 9, 8 ],
    [ 11, 0, 9, 4 ],
    [ 15, 0, 9, 15 ],
    [ 20, 0, 9, 4 ] ],
  [ [ 0, 0, 10, 3 ] ],
  [],
  [ [ 0, 0, 12, 2 ], [ 9, 0, 12, 9 ], [ 12, 0, 12, 2 ] ],
  [ [ 0, 0, 13, 1 ], [ 1, 0, 0, 0 ] ] ]

This doesn't seem at all what that _parseMappingsis expecting.

peteruithoven commented 7 years ago

I've been digging into this, by creating a bundle node script and using the "new" --inspect feature. https://medium.com/@paul_irish/debugging-node-js-nightlies-with-chrome-devtools-7c4a1b95ae27#.r4f59c70b

Looks like babel.transform is called with a inputSourceMap option which is the above mentioned array. https://github.com/systemjs/builder/blob/0.15.34/compilers/esm.js#L116

peteruithoven commented 7 years ago

In the compileTree method one of my packages contains a .metadata.sourceMap.mappings which is that array. While for most packages this contains a string like with something like:

;;;AAAA,SAAA,AAAS,yBAAT,AAAkC;AAClC,SAAA,AAAS,4BAAT,AAAqC;;AAErC,IAAM,iBAAN,AAAuB;;AAEvB,eAAe,SAAA,AAAS,cAAT,AAAuB,sBAAsB,AAC1D;sBAAA,AACK;AADL,4BAAA,AAES,MAAM;kBACX;;yBAAA;6DAAO,iBAAA,AAAO,UAAP,AAAiB,UAAjB;cAAA,AACC,OADD,AAEC,WAFD,AAIC,MAJD,AAMC;sEAND;sBAAA;+CAAA;qBACC;AADD,0BAAA,AACS,AACR;AAFD,8BAEa,kBAAkB,MAAA,AAAM,SAFrC,AAEa,AAAiC,AAE7C;AAJD,yBAIQ,EAAE,WAAF,WAAa,MAJrB,AAIQ,AAEP;AAND,iCAMgB,OAAA,AAAO,KAAP,AAAY,eAN5B,AAMgB,AAA2B;4DAEhC,eAAA,AAAe,OAAxB,AAAS,AAAsB,OAA/B,AAAsC,KAAK,gBAAyB;wBAAf,AAAe,gBAAtB,AAAsB,AACzE;;wBAAA,AAAI,cAAc,AAChB;mCAAA,AAAa,WAAW,SAAxB,AAAiC,AAClC;AAFD,2BAEO,AACL;;+BAAc,AAEZ;iCAFY,AAGZ;8BAAM,SAHM,AAGG,AACf;qCAJY,AAIC,AACb;kCALF,AAAS,AAAK,AAKF,AAEb;AAPe,AACZ,uBADO;AASX;;6BAAS,qBAAA,AAAqB,+CAA9B,AAAS,AAAoE,AAC9E;AAdM,mBAAA,EAAA,AAcJ,MAAM,iBAAS,AAChB;wBAAA,AAAI,cAAc,aAAA,AAAa,AAC/B;6BAAS,qBAAA,AAAqB,kDAArB,AAAqE,OAA9E,AAAS,AAA8E,AACxF;AAzBI,AAQE;;qBARF;qBAAA;kCAAA;;AAAA;sBAAA;AAAP;;kCAAA;iCAAA;AAAA;AA2BD;AA9BH,AAgCD;AA9BG

Looks like traceExpression is executed twice, where in the following location the first time the module where bundling fails has a regular string mapping, but the second time an array. https://github.com/systemjs/builder/blob/0.15.34/lib/arithmetic.js#L262

Actually, this makes me think it's caused by the Worker plugin we use. That this triggers two bundling/build steps where the second time it fails on things that where transpiled before. The file where the bundling fails is used both in a worker and outside a worker.

peteruithoven commented 7 years ago

The Worker plugin uses a map to check if it's in node or the browser (the intension was to detect if it's in production or development mode): https://github.com/casperlamboo/plugin-worker/blob/master/package.json#L14-L19 When it's in node it will call builder.buildStatic: https://github.com/casperlamboo/plugin-worker/blob/master/src/loader-production.js#L5

peteruithoven commented 7 years ago

By manually excluding the overlapping dependencies bundling works again. src/js/index.js - jspm_packages/npm/systemjs-plugin-babel@0.0.12/babel-helpers/defineProperty.js - src/js/utils/trace.js

@guybedford do you perhaps have any advice? Would it be doable to make builder.buildStatic and builder.bundle more robust, so that they can handle these kind of cases or is there a better way to do the Worker plugin? Perhaps this is fixed with SystemJS >0.19.41?

guybedford commented 7 years ago

@peteruithoven I think this may be because Babel mutates the input source map? If that is the case, then before passing the input source map to Babel, it may just need to be cloned into a new object to ensure it isn't mutated? This may be as simple as replacing the line at https://github.com/systemjs/builder/blob/0.15.34/compilers/esm.js#L98 with:

inputSourceMap: Object.assign({}, load.metadata.sourceMap)

or similar. Let me know if that sounds like something that might work for you here.

peteruithoven commented 7 years ago

That sounds interesting. I've tried your suggestion, but that doesn't fix the issue. Looking through the logging I placed in the code I can see it's already altered before it ever gets to that compile call.

For example, when I log nextTrace.tree['somespecificmodule'].metadata.sourceMap.mappings here: https://github.com/systemjs/builder/blob/0.15.34/lib/arithmetic.js#L262 The first time it's a string, the second time it's an array.

So there is probably some earlier preparation step where we need to clone the sourceMap.