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

Compile Plugins written in Typescript #57

Conversation

mcMickJuice
Copy link
Contributor

#51

@mcMickJuice
Copy link
Contributor Author

Should we write a test that ensures that a friendly error message is emitted if typescript is not available?

@eventualbuddha
Copy link
Collaborator

Should we write a test that ensures that a friendly error message is emitted if typescript is not available?

Probably, though since typescript is installed it might be a little tricky.

@mcMickJuice
Copy link
Contributor Author

Hmm, we could modify Module._resolveFilename to throw if that module being required is 'typescript'. However we'd need to setup this require hook in the context of the cli process.

It's a little dirty but we can have a separate cli command for this test:

const run = require('../../').default;
const Module = require('module');

const originalLoad = Module._resolveFilename
Module._resolveFilename = function monkeyPatchedLoad(...args) {
  const [path] = args;

  if(path === 'typescript'){
    throw new Error();
  }

  return originalLoad(...args)
}

run(process.argv, process.stdin, process.stdout, process.stderr)
  .then(status => {
    process.exit(status);
  })
  .catch(err => {
    console.error(err.stack);
    process.exit(-1);
  });

Then run this file for our test, inspecting the error. Any value in this though?

@eventualbuddha
Copy link
Collaborator

I tried rebasing this but ran into some trouble. Let me know if you want me to push up my rebase or if you want to take a crack at it.

@mcMickJuice
Copy link
Contributor Author

Ok. I can take a crack at it later this week.

@eventualbuddha
Copy link
Collaborator

One other approach to consider: should we use Babel to transpile TypeScript? I know Babel has support for parsing and transforming TypeScript, but I don’t know how well supported that is. @hzoo, do you know?

@mcMickJuice
Copy link
Contributor Author

I did a rebase with not many issues, however i have 3 tests in the OptionsTests fixture failing. They're very strange issues...

  3) Options
       has sensible defaults:
     AssertionError: undefined deepEqual []
      at Context.<anonymous> (test/OptionsTest.ts:9:5)

  4) Options
       associates plugin options based on declared name:
     Error: expected plugin to be [plugin, options] tuple: Promise { <pending> }
      at Context.<anonymous> (test/OptionsTest.ts:71:13)

  5) Options
       associates plugin options based on inferred name:
     Error: expected plugin to be [plugin, options] tuple: Promise { <pending> }
      at Context.<anonymous> (test/OptionsTest.ts:100:13)

Did you run into this after performing a rebase?

@eventualbuddha
Copy link
Collaborator

eventualbuddha commented Jan 18, 2018

I ran into different issues. The second and third ones look like you’re missing an await on the result of an async function call. Not sure about the first. If you push what you have I can take a look.

@mcMickJuice
Copy link
Contributor Author

so bizarre. When running the OptionsTest suite in isolation, the tests pass. When running the whole suite of tests, they fail. Looking into this this morning

@mcMickJuice
Copy link
Contributor Author

bah. i had old test .js files

@mcMickJuice mcMickJuice force-pushed the feature/typescript-compile-plugins branch from c5f42b7 to c8f00c0 Compare January 19, 2018 14:53
@mcMickJuice
Copy link
Contributor Author

OK, i rebased, got tests to pass and updated how transpiling via typescript modifies the resolve extensions in FileSystemResolver. Last thing is, if we want, write a test to output friendly error message if typescript is not installed. I tried the following:

Write a script that executes codemod bin in a context where requiring typescript throws an error:

#!/usr/bin/env node

const run = require('../../').default;
const Module = require('module');

const originalLoad = Module._resolveFilename
Module._resolveFilename = function monkeyPatchedLoad(...args) {
  const [path] = args;

  if(path === 'typescript'){
    throw new Error();
  }

  return originalLoad(...args)
}

run(process.argv, process.stdin, process.stdout, process.stderr)
  .then(status => {
    process.exit(status);
  })
  .catch(err => {
    console.error(err.stack);
    process.exit(-1);
  });

This file is placed in /test/helpers/codemodCLIWithNoTypescript file. I execute this file the same way we executee runCodemodCLI:

export async function runCodemodCLIWithoutTypescript(
  args: Array<string>,
  stdin?: string
): Promise<CLIResult> {
  return new Promise(
    (resolve: (result: CLIResult) => void, reject: (error: Error) => void) => {
      let child = execFile(join(__dirname, './CodemodCLIWithNoTypescript'), args);
      let stdout = '';
      let stderr = '';

      child.stdin.end(stdin);

      child.on('close', status => {
        resolve({ status, stdout, stderr });
      });

      child.stdout.on('data', chunk => {
        stdout += chunk;
      });

      child.stderr.on('data', chunk => {
        stderr += chunk;
      });

      child.on('error', reject);
    }
  );
} 

And in CLI test:

//test setup

    let { status, stdout, stderr } = await runCodemodCLIWithoutTypescript([afile, '-p', plugin('typescript/increment-export-default-multiple/index', '.ts'), '--transpile-ts-plugins']);

//assertions

However, running this tests results in a spawn EACCES error. Is this because of the location of the codemodCLIWithNoTypescript script? I'm not familiar with how node execFile and file permissions work.

What are your thoughts on this? If can push these changes to the repo if you wanna inspect this. Let me know.

@mcMickJuice
Copy link
Contributor Author

re: #57 (comment)

Typescript through babel seems to be possible but I think it's still in alpha? Here's the issue at least and its resolution:

babel/babylon#320 (comment)

I guess @hzoo would obviously be the best person to answer this

@hzoo
Copy link

hzoo commented Jan 19, 2018

Yep that was a really old issue, we've since made a lot more changes/fixes - https://github.com/babel/babel/labels/area%3A%20typescript in active development so we know people are using it.

@eventualbuddha
Copy link
Collaborator

Some of this was incorporated in #79 which uses @babel/preset-typescript to process TypeScript, meaning we don't actually need to have a local typescript package installed. Thanks for your work on this, @mcMickJuice!

@mcMickJuice mcMickJuice deleted the feature/typescript-compile-plugins branch January 24, 2018 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants