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

Document that we're using Babel #274

Closed
vjeux opened this issue Mar 26, 2015 · 15 comments · Fixed by #1449
Closed

Document that we're using Babel #274

vjeux opened this issue Mar 26, 2015 · 15 comments · Fixed by #1449
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@vjeux
Copy link
Contributor

vjeux commented Mar 26, 2015

https://github.com/facebook/jstransform/tree/master/visitors

@glenjamin
Copy link

Aha, I was just trying to find this out!

Is this optional, is it possible to swap in babel or similar?

@vjeux
Copy link
Contributor Author

vjeux commented Mar 26, 2015

Right now it's backed in but we want to allow other transforms down the line. We secretly hoped that someone from the community would help :)

@amasad
Copy link
Contributor

amasad commented Mar 26, 2015

All the transformation is done by the worker https://github.com/facebook/react-native/blob/master/packager/transformer.js
Might be able to just hack it in their and it should work. (source maps is a bit trickier for now)

@glenjamin
Copy link

Once the initial rush settles down and the issues take some shape it'd be good to have a bit of a roadmap for larger goals like that - I'm keen to get stuck in :)

@vishnevskiy
Copy link
Contributor

I managed to swap in babel and it works 95%.

For some reason even though it transforms import statements to requires they don't seem to work. Does react-native crawl the requires BEFORE the transform occurs?

@amasad
Copy link
Contributor

amasad commented Mar 27, 2015

Yes, I'd say avoid es6 modules for now. Can you share your code for others
to see?

On Thursday, March 26, 2015, Stanislav Vishnevskiy notifications@github.com
wrote:

I managed to swap in babel and it works 95%.

For some reason even though it transforms import statements to requires
they don't seem to work. Does react-native crawl the requires BEFORE the
transform occurs?


Reply to this email directly or view it on GitHub
#274 (comment)
.

@vishnevskiy
Copy link
Contributor

No problem.

I just installed babel-core and replaced transformer.js in packager with.

'use strict';

var babel = require('babel-core');

module.exports = function(data, callback) {
  var result;
  try {
    result = babel.transform(data.sourceCode, {
      filename: data.filename,
      // Remove the interopRequire
      modules: 'commonStrict'
    });
    result = {
      // Removing interopRequire causes it to try to get default.
      // Not nessessary for react-native so strip it.
      code: result.code.replace(/\)\["default"]/g, ')')
    };
  } catch (e) {
    return callback(null, {
      error: {
        lineNumber: e.loc.line,
        column: e.loc.column,
        message: e.message,
        stack: e.stack,
        description: e.description
      }
    });
  }

  callback(null, result);
};

I don't seem to see how to provide the source map but it works otherwise.

I would really love advise how where I might be able to mod code to fix import statements because that is blocking me from using existing Flux stores and action creators from another react project.

@sebmck
Copy link
Contributor

sebmck commented Mar 27, 2015

Why are you using commonStrict?

@amasad
Copy link
Contributor

amasad commented Mar 27, 2015

Yup souremaps are currently hardcoded in the packager for speed and assume line/col numbers are preserved (jstransform) (this hack made things orders of magnitude faster for us) because source map generation is usually the most expensive part of packagers. But will try to have a better answer for this soon.

@vishnevskiy
Copy link
Contributor

@sebmck For some reason the packager does not like the _interopRequire

@pilwon
Copy link
Contributor

pilwon commented Mar 28, 2015

Here's my solution:

Add the following code to the top of packager/transformer.js's module.exports function:

var babel = require('babel');
if (!/node_modules\/react-native\//.test(data.filename)) {
  try {
    var result = babel.transform(data.sourceCode, {
      ast: false,
      comments: false,
      filename: data.filename
    });
    return callback(null, {
      code: result.code
    });
  } catch (e) {
    return callback(null, {
      error: {
        lineNumber: e.loc.line,
        column: e.loc.column,
        message: e.message,
        stack: e.stack,
        description: e.description
      }
    });
  }
}

It transforms node_modules/react-native/**/* with the default jstransform transformer and sends the rest to babel. With this approach, I didn't have interopRequire problem @vishnevskiy mentioned above.

I was also able to solve the ES6 import statement problem by patching DependencyResolver's DependencyGraph module. (Submitted PR #386)

Please share if you discover any issue other than the known issue with sourcemap with this approach.

@jonrh
Copy link

jonrh commented Mar 28, 2015

Yes please! I spent about an hour trying to find this documentation, what ES6 features I could use and what not. Thanx @tadeuzagallo for pointing me to this issue.

@ide ide changed the title Document that we're using JSTransform Document that we're using Babel May 28, 2015
@jsierles
Copy link
Contributor

Working on a PR for documentation.

@jsierles
Copy link
Contributor

PR for review at #1449. If we want this documented before the official release is out, I can update it to reference both transformers.

@amasad
Copy link
Contributor

amasad commented Jun 2, 2015

FYI the packager can now consume map sourcemaps returned from the transformer script

@facebook facebook locked as resolved and limited conversation to collaborators May 31, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants