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

Add and test source map support #23

Closed
mjmeintjes opened this Issue Jan 19, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@mjmeintjes
Owner

mjmeintjes commented Jan 19, 2016

Source maps are not currently being picked up correctly.

https://github.com/mjmeintjes/boot-react-native/blob/master/resources/mattsum/boot_rn/js/cljs-rn-transformer.js#L19
RN might be expecting source map contents instead of file name?

@scttnlsn

This comment has been minimized.

Show comment
Hide comment
@scttnlsn

scttnlsn Jan 20, 2016

Played around with this a bit but had no luck with providing the map contents or filename. Hoping for a response here: facebook/react-native#393 (comment)

scttnlsn commented Jan 20, 2016

Played around with this a bit but had no luck with providing the map contents or filename. Hoping for a response here: facebook/react-native#393 (comment)

@scttnlsn

This comment has been minimized.

Show comment
Hide comment
@scttnlsn

scttnlsn Jan 20, 2016

I tried

const fs = require('fs');
const transformer = require('react-native/packager/transformer');

function transform(code, filename, callback) {
  fs.readFile(filename + '.map', function (err, map) {
    callback(null, {
      code: code,
      map: err ? null : map.toString()
    });
  });
}

module.exports = function (data, callback) {
  if (data.filename && data.filename.indexOf('/build/') > -1) {
    transform(data.sourceCode, data.filename, callback);
  } else {
    transformer(data, callback);
  }
};

and

...

callback(null, {
  code: code,
  map: err ? null : JSON.parse(map.toString())
});

...

but still get index.io.bundle in every line of the stack traces.

scttnlsn commented Jan 20, 2016

I tried

const fs = require('fs');
const transformer = require('react-native/packager/transformer');

function transform(code, filename, callback) {
  fs.readFile(filename + '.map', function (err, map) {
    callback(null, {
      code: code,
      map: err ? null : map.toString()
    });
  });
}

module.exports = function (data, callback) {
  if (data.filename && data.filename.indexOf('/build/') > -1) {
    transform(data.sourceCode, data.filename, callback);
  } else {
    transformer(data, callback);
  }
};

and

...

callback(null, {
  code: code,
  map: err ? null : JSON.parse(map.toString())
});

...

but still get index.io.bundle in every line of the stack traces.

@mjmeintjes

This comment has been minimized.

Show comment
Hide comment
@mjmeintjes

mjmeintjes Jan 20, 2016

Owner

Some ideas

Here's a document describing the source map format:
https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit

(1) I just looked at one of the actual source map files generated, and it seems that the "file" property is pointing to the boot temp directory where it was generated, which doesn't exist anymore after build is done:

{
   "version":3,
   "file":"\/home\/matthys\/.boot\/cache\/tmp\/home\/matthys\/projects\/boot-react-native\/example\/4er\/p9tkpy\/main.out\/adzerk\/boot_cljs_repl.js",
   "sources":[
      "boot_cljs_repl.cljs"
   ],
   "lineCount":17,
   "mappings":";AAAA;;;AACA,qBAAA,jBAAMA;AAAN,AAAuC,oBAAM,iBAAAC,oBAAKD;AAAL,AAAA,oBAAAC;AAAe,OAACC,wBAAI,AAACC;;AAArBF;;;AAAN,AAA0C,8BAAA,9BAACG;;AAA3C",
   "names":[
      "repl-conn",
      "and__4976__auto__",
      "cljs.core\/not",
      "weasel.repl\/alive?",
      "weasel.repl\/connect"
   ]
}

Maybe if we update the "file" path to point to the new "node_modules/MODULE-NAME.js". Not sure if this actually gets used anywhere?

(2) It might also be worth loading the actual content of the module into the "sourcesContent" property.

(3) Alternatively, maybe setting the "sourceRoot" property to "/build/node_modules"

Owner

mjmeintjes commented Jan 20, 2016

Some ideas

Here's a document describing the source map format:
https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit

(1) I just looked at one of the actual source map files generated, and it seems that the "file" property is pointing to the boot temp directory where it was generated, which doesn't exist anymore after build is done:

{
   "version":3,
   "file":"\/home\/matthys\/.boot\/cache\/tmp\/home\/matthys\/projects\/boot-react-native\/example\/4er\/p9tkpy\/main.out\/adzerk\/boot_cljs_repl.js",
   "sources":[
      "boot_cljs_repl.cljs"
   ],
   "lineCount":17,
   "mappings":";AAAA;;;AACA,qBAAA,jBAAMA;AAAN,AAAuC,oBAAM,iBAAAC,oBAAKD;AAAL,AAAA,oBAAAC;AAAe,OAACC,wBAAI,AAACC;;AAArBF;;;AAAN,AAA0C,8BAAA,9BAACG;;AAA3C",
   "names":[
      "repl-conn",
      "and__4976__auto__",
      "cljs.core\/not",
      "weasel.repl\/alive?",
      "weasel.repl\/connect"
   ]
}

Maybe if we update the "file" path to point to the new "node_modules/MODULE-NAME.js". Not sure if this actually gets used anywhere?

(2) It might also be worth loading the actual content of the module into the "sourcesContent" property.

(3) Alternatively, maybe setting the "sourceRoot" property to "/build/node_modules"

@scttnlsn

This comment has been minimized.

Show comment
Hide comment
@scttnlsn

scttnlsn Jan 21, 2016

The temp files still exist for me and, according to the spec, I understand the sources should be resolved relative to the file (if the sources are not absolute URLs). Not sure I understand the build/node_modules directory. Why aren't those files nested under build/main.out? I would have expected node_modules to contain JavaScript modules installed via NPM.

scttnlsn commented Jan 21, 2016

The temp files still exist for me and, according to the spec, I understand the sources should be resolved relative to the file (if the sources are not absolute URLs). Not sure I understand the build/node_modules directory. Why aren't those files nested under build/main.out? I would have expected node_modules to contain JavaScript modules installed via NPM.

@mjmeintjes

This comment has been minimized.

Show comment
Hide comment
@mjmeintjes

mjmeintjes Jan 22, 2016

Owner

Hi

I think I've gotten the source mapping to work (with a few caveats, see comments).

The reason for build/node_modules is that boot-react-native copies and renames modules generated by CLJS in order to work with React Native packager. See here for a more in-depth explanation: https://github.com/mjmeintjes/boot-react-native/blob/master/src/mattsum/boot_react_native.clj#L15

Here's the code for the updated transformer. It turned out to be a little more complicated, because once you return the .map property for a module, RN starts to expect that property for all modules. Unfortunately, the base RN transformer does not provide the .map property, which causes Chrome to fail with 'sources' not found on undefined.

What I've done here is try to generate the .map property for all modules. This works on my PC for CLJS files, however Chrome does not seem to pick up the source maps for the JS files - luckily it falls back to just displaying the bundled JS, which is enough for our purpose.

/**
 * Copyright (c) 2015-present, Facebook, Inc.
 * All rights reserved.
 *
 * This source code is licensed under the BSD-style license found in the
 * LICENSE file in the root directory of this source tree. An additional grant
 * of patent rights can be found in the PATENTS file in the same directory.
 *
 * Note: This is a fork of the fb-specific transform.js
 */
'use strict';

// TODO: this should probably not be hardcoded
const fs = require('fs');
const transformer = require('react-native/packager/transformer');

const util = require('util');
function getSourceMapModuleName(code) {
    //Extract the source map module name from source code
    var sourceMapRegex = /sourceMappingURL=(.*)\.js\.map/;
    var match = sourceMapRegex.exec(code);
    var moduleName = "";
    if (match != null) {
          moduleName = match[1] + ".cljs";
    }
    return moduleName;
}
function transform(code, filename, callback) {
    fs.readFile(filename + '.map', function (err, map) {
        var sourceMap = err ?
                getBasicSourceMap(filename, code) :
                JSON.parse(map.toString());

        sourceMap.sources = [getSourceMapModuleName(code)];

        //Hardcode the root where our cljs files have been moved to
        sourceMap.sourceRoot = "/build/node_modules/";
        //Alternatively we could set sourcesContent to [<CONTENT OF CLJS FILE>]

        callback(null, {
            code: code.replace("# sourceMappingURL=", ""),
            map: sourceMap
        });
    });
}

function getBasicMappings(code) {
    //TODO: This is copied from https://github.com/facebook/react-native/blob/528e30987aba8848f8c8815f00c42ecb2ce0919a/packager/react-packager/src/Bundler/Bundle.js#L265
    //but unfortunately I know next to nothing about setting up source maps.
    //Therefore it is not working. But, this is only run for js files, and Chrome falls
    //back to displaying the Javascript from the bundled file anyway when it can't understand
    //the source mappings from here.
    //TLDR - I don't think this does anything, and it probably needs to be fixed, but not critical.
    var mappings = "";
    const line = 'AACA';
    let lastCharNewLine  = false;
    var moduleLines = 0;
    for (let t = 0; t < code.length; t++) {
        if (t === 0) {
            mappings += 'AC';
            mappings += 'A';
        } else if (lastCharNewLine) {
            moduleLines++;
            mappings += line;
        }
        lastCharNewLine = code[t] === '\n';
        if (lastCharNewLine) {
            mappings += ';';
        }
    }
    mappings += ';';
    return mappings;
}
function getBasicSourceMap(filename, code) {
    //This is supposed to just provide a fallback sourcemap
    //If we don't provide a source map for every module, then Google Chrome
    //starts throwing errors ('sources' could not be found on undefined)
    //Chrome expects every section (and there is a section for each module) to
    //have a 'map' property with an array of 'sources'.

    var mappings = getBasicMappings(code);
    const map = {
        file: filename,
        sources: [filename],
        version: 3,
        names: [],
        mappings: mappings,
        sourcesContent: [code]
    };
    return map;
}

module.exports = function (data, callback) {
    if (data.sourceCode && data.sourceCode.indexOf('Compiled by ClojureScript') > -1) {
        transform(data.sourceCode, data.filename, callback);
    } else {
        var cb = function(err, mod) {
            mod.map = getBasicSourceMap(data.filename, mod.code);
            return callback(err,  mod);
        };
        transformer(data, cb);
    }
};
Owner

mjmeintjes commented Jan 22, 2016

Hi

I think I've gotten the source mapping to work (with a few caveats, see comments).

The reason for build/node_modules is that boot-react-native copies and renames modules generated by CLJS in order to work with React Native packager. See here for a more in-depth explanation: https://github.com/mjmeintjes/boot-react-native/blob/master/src/mattsum/boot_react_native.clj#L15

Here's the code for the updated transformer. It turned out to be a little more complicated, because once you return the .map property for a module, RN starts to expect that property for all modules. Unfortunately, the base RN transformer does not provide the .map property, which causes Chrome to fail with 'sources' not found on undefined.

What I've done here is try to generate the .map property for all modules. This works on my PC for CLJS files, however Chrome does not seem to pick up the source maps for the JS files - luckily it falls back to just displaying the bundled JS, which is enough for our purpose.

/**
 * Copyright (c) 2015-present, Facebook, Inc.
 * All rights reserved.
 *
 * This source code is licensed under the BSD-style license found in the
 * LICENSE file in the root directory of this source tree. An additional grant
 * of patent rights can be found in the PATENTS file in the same directory.
 *
 * Note: This is a fork of the fb-specific transform.js
 */
'use strict';

// TODO: this should probably not be hardcoded
const fs = require('fs');
const transformer = require('react-native/packager/transformer');

const util = require('util');
function getSourceMapModuleName(code) {
    //Extract the source map module name from source code
    var sourceMapRegex = /sourceMappingURL=(.*)\.js\.map/;
    var match = sourceMapRegex.exec(code);
    var moduleName = "";
    if (match != null) {
          moduleName = match[1] + ".cljs";
    }
    return moduleName;
}
function transform(code, filename, callback) {
    fs.readFile(filename + '.map', function (err, map) {
        var sourceMap = err ?
                getBasicSourceMap(filename, code) :
                JSON.parse(map.toString());

        sourceMap.sources = [getSourceMapModuleName(code)];

        //Hardcode the root where our cljs files have been moved to
        sourceMap.sourceRoot = "/build/node_modules/";
        //Alternatively we could set sourcesContent to [<CONTENT OF CLJS FILE>]

        callback(null, {
            code: code.replace("# sourceMappingURL=", ""),
            map: sourceMap
        });
    });
}

function getBasicMappings(code) {
    //TODO: This is copied from https://github.com/facebook/react-native/blob/528e30987aba8848f8c8815f00c42ecb2ce0919a/packager/react-packager/src/Bundler/Bundle.js#L265
    //but unfortunately I know next to nothing about setting up source maps.
    //Therefore it is not working. But, this is only run for js files, and Chrome falls
    //back to displaying the Javascript from the bundled file anyway when it can't understand
    //the source mappings from here.
    //TLDR - I don't think this does anything, and it probably needs to be fixed, but not critical.
    var mappings = "";
    const line = 'AACA';
    let lastCharNewLine  = false;
    var moduleLines = 0;
    for (let t = 0; t < code.length; t++) {
        if (t === 0) {
            mappings += 'AC';
            mappings += 'A';
        } else if (lastCharNewLine) {
            moduleLines++;
            mappings += line;
        }
        lastCharNewLine = code[t] === '\n';
        if (lastCharNewLine) {
            mappings += ';';
        }
    }
    mappings += ';';
    return mappings;
}
function getBasicSourceMap(filename, code) {
    //This is supposed to just provide a fallback sourcemap
    //If we don't provide a source map for every module, then Google Chrome
    //starts throwing errors ('sources' could not be found on undefined)
    //Chrome expects every section (and there is a section for each module) to
    //have a 'map' property with an array of 'sources'.

    var mappings = getBasicMappings(code);
    const map = {
        file: filename,
        sources: [filename],
        version: 3,
        names: [],
        mappings: mappings,
        sourcesContent: [code]
    };
    return map;
}

module.exports = function (data, callback) {
    if (data.sourceCode && data.sourceCode.indexOf('Compiled by ClojureScript') > -1) {
        transform(data.sourceCode, data.filename, callback);
    } else {
        var cb = function(err, mod) {
            mod.map = getBasicSourceMap(data.filename, mod.code);
            return callback(err,  mod);
        };
        transformer(data, cb);
    }
};
@mjmeintjes

This comment has been minimized.

Show comment
Hide comment
@mjmeintjes

mjmeintjes Jan 22, 2016

Owner

I've pushed the changes I made to sourcemap branch.

Owner

mjmeintjes commented Jan 22, 2016

I've pushed the changes I made to sourcemap branch.

@mjmeintjes

This comment has been minimized.

Show comment
Hide comment
@mjmeintjes

mjmeintjes Feb 15, 2016

Owner

Fixed by #35

Owner

mjmeintjes commented Feb 15, 2016

Fixed by #35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment