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

es6 modules - property imports #6073

Merged
merged 1 commit into from
Feb 3, 2016

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Feb 2, 2016

This change is slightly more complex than #6072, it converts all require statements that referenced a sub-property of a module.

@epixa
Copy link
Contributor

epixa commented Feb 2, 2016

Test failure on the destructuring commit

@spalger spalger assigned epixa and unassigned spalger Feb 2, 2016
@spalger
Copy link
Contributor Author

spalger commented Feb 2, 2016

@epixa do you think it's worth doing this in multiple prs?

@epixa
Copy link
Contributor

epixa commented Feb 2, 2016

I definitely do, yeah. The changes are fundamentally not complex, but there are so many of them...

@spalger
Copy link
Contributor Author

spalger commented Feb 2, 2016

Cool

@epixa
Copy link
Contributor

epixa commented Feb 2, 2016

There's a different test failure this time. You can rebase this as well to knock out the first commit.

@epixa epixa assigned spalger and unassigned epixa Feb 2, 2016
@spalger spalger changed the title destructured es6 modules destructured and direct es6 modules Feb 2, 2016
@spalger spalger changed the title destructured and direct es6 modules destructured es6 modules Feb 2, 2016
@spalger spalger force-pushed the implement/es6Modules/destructured branch from 3e60b49 to ba19f89 Compare February 2, 2016 23:20
@kimjoar
Copy link
Contributor

kimjoar commented Feb 2, 2016

I played around with jscodeshift some time ago and tried this on Kibana:

module.exports = function(fileInfo, api) {
  var j = api.jscodeshift;
  var root = j(fileInfo.source);

  root
    .find(j.VariableDeclaration, {
      declarations: [{
        type: 'VariableDeclarator',
        init: {
          type: 'CallExpression',
          callee: {
            type: 'Identifier',
            name: 'require',
          },
        },
      }],
    })
    .filter(isTopLevel)
    .forEach(function(path) {
      const dec = path.value.declarations[0];
      const id = dec.id;
      const source = dec.init.arguments[0];
      const comments = path.value.comments;
      const loc = path.value.loc;

      path.replace(
        j.importDeclaration(
          [{
            type: 'ImportDefaultSpecifier',
            id
          }],
          source
        )
      );
      path.value.loc = loc;
      path.value.comments = comments;
    });

  root
    .find(j.VariableDeclaration, {
      declarations: [{
        type: 'VariableDeclarator',
        init: {
          type: 'MemberExpression',
          object: {
            type: 'CallExpression',
            callee: {
              type: 'Identifier',
              name: 'require'
            },
          },
        },
      }],
    })
    .filter(isTopLevel)
    .forEach(function(path) {
      const dec = path.value.declarations[0];
      const name = dec.id;
      const source = dec.init.object.arguments[0];

      // We cannot transform require's which include function calls
      if (source.type === 'CallExpression') return;

      const id = dec.init.property;
      const comments = path.value.comments;
      const loc = path.value.loc;

      let spec = {
        type: 'ImportSpecifier',
        id,
      }
      if (name.name !== id.name) {
        spec['name'] = name;
      }

      path.replace(j.importDeclaration([spec], source));
      path.value.loc = loc;
      path.value.comments = comments;
    });

  return root.toSource();
};

function isTopLevel(path) {
  return !path.parentPath.parentPath.parentPath.parentPath;
}

and

jscodeshift -t kibana-transform.js src/ui/**/*.js

Seemed to work ok. Fun to play around with. Totally forgot about it, therefore no PR 🙈 (Thought I'd just mention jscodeshift as a fun tool when transforming code)

Btw, http://astexplorer.net/ helps a lot when playing around with this.

@spalger
Copy link
Contributor Author

spalger commented Feb 2, 2016

interesting, I played around with babel before going down the route I have now but the output messed with formatting too much. this seems to handle the require -> import really nicely. I wonder if I could use it to convert files where the require is passed directly to Private().

@kimjoar
Copy link
Contributor

kimjoar commented Feb 2, 2016

I struggled a bit with it, so it probably takes some fighting to get there ;) Might be some inspiration to get in https://github.com/5to6/5to6-codemod and https://github.com/rackt/rackt-codemod

@spalger spalger assigned epixa and unassigned spalger Feb 2, 2016
@spalger spalger changed the title destructured es6 modules es6 modules - property imports Feb 2, 2016
@spalger spalger force-pushed the implement/es6Modules/destructured branch from ba19f89 to c65e136 Compare February 3, 2016 17:36
@spalger spalger force-pushed the implement/es6Modules/destructured branch from c65e136 to f37e0f4 Compare February 3, 2016 17:38
@epixa
Copy link
Contributor

epixa commented Feb 3, 2016

LGTM

@epixa epixa assigned spalger and unassigned epixa Feb 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants