Adds option for running plugin through babel-preset-env pipeline #31

mcMickJuice · 2017-06-25T05:39:00Z

Points of discussion/questions:

Not sure of naming of this cli option
How can we write a unit test against this?

eventualbuddha · 2017-12-22T20:49:46Z

@mcMickJuice sorry it took me so long to get to this. I've mostly finished it up, leaving the option name the same as you had it. I added a test to ensure export default … plugins work as expected. What do you think?

mcMickJuice · 2017-12-25T14:44:37Z

No problem and all looks good. Ship it!

eventualbuddha · 2017-12-25T17:29:11Z

Cool, still some things I need to do first:

update help docs to encourage --babel-transform instead of babel-register
~~add an equivalent option for TypeScript?~~ (Add option to transpile TypeScript plugins directly #51)
ensure plugins with multiple files get transpiled correctly

mcMickJuice · 2017-12-28T21:48:39Z

For typescript, should we have a separate issue to tackle that? along with doc updates and tests?

eventualbuddha · 2017-12-29T13:11:25Z

Yeah, I’ll make a ticket for TypeScript.

mcMickJuice · 2017-12-29T14:23:17Z

update help docs to encourage --babel-transform instead of babel-register

Updated readme, explaining how to use new switch and when to use babel-register instead of --babel-transpile. Split out example of using ts-node/register

mcMickJuice · 2017-12-29T15:14:47Z

ensure plugins with multiple files get transpiled correctly

Currently working on this and my test is failing. Seems like imported modules are not being transpiled...

eventualbuddha · 2017-12-29T15:30:56Z

Seems like imported modules are not being transpiled...

Yeah, I didn’t really explain it in my comment, but that’s something that I was reasonably sure wouldn’t work correctly with the way I implemented it. The problem is that the function I pass to babel-register to determine which files should be transpiled only whitelists the path that was require.resolve-d, but nothing imported or required by that file.

A better approach might be to do the require.resolve, then somehow whitelist everything in that package (if it is a package) or in its directory, possibly excluding node_modules and other stuff like that. Unfortunately, this is somewhat complex and I’m open to alternative suggestions. For example, what about doing the require inside a try block and, if the error is a SyntaxError, turn on babel-register for everything and try again?

mcMickJuice · 2017-12-29T15:51:51Z

Yep, removing the ignore function fixes this issue.

Are you looking to avoid transpiling node_modules for purposes of speed? If so, babel-register does this by default.

mcMickJuice · 2017-12-29T16:02:36Z

We could also just accept an ignore regex in the event that someone is using an npm package that is not transpiled. Essentially exposing the same config setting that babel-register accepts

eventualbuddha · 2017-12-29T16:14:02Z

By default babel-register also only transpiles files in packages that have babel-register as a dependency, right? I think that’s one issue I was trying to avoid eight the approach I took.

mcMickJuice · 2017-12-29T16:32:17Z

I'm not sure about that. But could we consider this an edge case and not alter babel-register's default behavior? If there are future issues we can have a bug opened and add further extension points (e.g. exposing babel-register config to cli).

If you're cool with this, I can remove the whitelisting and push the test I added.

eventualbuddha · 2017-12-30T14:17:48Z

Sure, let’s start with this and file bugs for other issues.

eventualbuddha · 2017-12-30T14:44:48Z

What do you think about making this on by default?

mcMickJuice · 2017-12-30T16:00:43Z

Completely agree. I just updated docs to say this functionality is on by default. I'll move forward with turning this on by default and exposing a switch to use babel-register.

Since this project now has a dependency on babel-register, should the switch be --use-local-babel which bypasses the registering of babel-preset-env and just calls babel-register without any arguments. If that sounds good to you I can start on that.

mcMickJuice · 2017-12-30T16:01:35Z

Not sure why my pushes are failing in the CI env 🤷‍♂️

mcMickJuice · 2017-12-30T16:53:23Z

Updated to use --use-local-babel switch and updated functionality to opt out of using babel-preset-env, updated tests and docs. Couldn't think of a good way to write a test to ensure that local babelrc file was being used if --use-local-babel switch is passed

eventualbuddha

Overall looking good. I will wait to merge until I get home and can test it out a bit on a development computer (I’m writing this on an iPad). Thanks for helping to get this feature in!

eventualbuddha · 2017-12-30T17:03:32Z

src/index.ts

@@ -15,6 +15,7 @@ OPTIONS
  -o, --plugin-options PLUGIN=OPTS  JSON-encoded OPTS for PLUGIN (allows multiple).
  -r, --require PATH                Require PATH before transform (allows multiple).
      --extensions EXTS             Comma-separated extensions to process (default: "${Array.from(DEFAULT_EXTENSIONS).join(',')}").
+      --use-local-babel             Run plugins through babel plugins/presets specified in local .babelrc file instead of babel-preset-env


nit: end this statement in a period.

Also, should we add a --no-transpile-plugins option? Using babel-register does slow things down a bit and it might be nice to allow opting out of that. What do you think?

is this something we can have as a separate, dependent issue on this feature? Just to keep this PR focused?

eventualbuddha · 2017-12-30T17:04:46Z

test/fixtures/plugin/increment-export-default-multiple/increment-value.js

@@ -0,0 +1,7 @@
+import path from 'path'
+
+const i = path.join(__dirname, 'hello.js')


Why is this here?

eventualbuddha · 2017-12-30T17:05:15Z

test/fixtures/plugin/increment-export-default-multiple/increment-value.js

+const i = path.join(__dirname, 'hello.js')
+
+export default function incrementValue(value) {
+    return value + 1


nit: 2-space indent, use semicolons. (I should set up prettier on this project)

eventualbuddha · 2017-12-30T17:05:34Z

test/fixtures/plugin/increment-export-default-multiple/increment-value.js

+
+export default function incrementValue(value) {
+    return value + 1
+}


nit: use trailing newlines

eventualbuddha · 2017-12-30T17:09:03Z

test/fixtures/plugin/increment-export-default-multiple/increment-export-default.js

+import incrementValue from './increment-value'
+
+export default function() {
+    return {


nit: use 2-space indent

eventualbuddha · 2017-12-30T17:10:32Z

test/CLITest.ts

+    let afile = await createTemporaryFile('a-file.js', '3 + 4;');
+    let { status } = await runCodemodCLI([afile, '-p', plugin('increment-export-default'), '--use-local-babel']);
+
+    const syntaxError = 255;


I get where you’re coming from with having this as const, but let’s just use let.

eventualbuddha · 2017-12-30T17:12:06Z

test/CLITest.ts

+
+  it('fails when specifying --use-local-babel as there are no plugins loaded', async function() {
+    let afile = await createTemporaryFile('a-file.js', '3 + 4;');
+    let { status } = await runCodemodCLI([afile, '-p', plugin('increment-export-default'), '--use-local-babel']);


We should also make some sort of assertion about the output. Ideally we could even catch SyntaxError and provide a better error message telling users they may need to use a custom .babelrc.

It's difficult to find where to place the try catch to determine if the SyntaxError is a result of the plugin or the code that we're modding. We could also look at the stack trace to determine where the syntax error occurred?:

/Users/z002s72/dev/git-clones/babel-codemod/test/fixtures/plugin/increment-export-default.js:1 (function (exports, require, module, __filename, __dirname) { export default function() { ^^^^^^ SyntaxError: Unexpected token export at createScript (vm.js:80:10) at Object.runInThisContext (vm.js:139:10) at Module._compile (module.js:599:28) at loader (/Users/z002s72/dev/git-clones/babel-codemod/node_modules/babel-register/lib/node.js:144:5) at Object.require.extensions.(anonymous function) [as .js] (/Users/z002s72/dev/git-clones/babel-codemod/node_modules/babel-register/lib/node.js:154:7) at Module.load (module.js:554:32) at tryModuleLoad (module.js:497:12) at Function.Module._load (module.js:489:3) at Module.require (module.js:579:17) at require (internal/module.js:11:18)

Looks like wrapping the following in try/catch captures the specific plugin related SyntaxError:

// index.ts, line 76 let plugins = options.getBabelPlugins();

However the error is undefined...im little lose here 😄

I’ll take a look when I get home.

eventualbuddha · 2017-12-30T17:13:05Z

src/Options.ts

@@ -83,6 +92,16 @@ export default class Options {
    }
  }

+  loadBabelTranspile() {
+    let pluginOptions;
+    if(!this.useLocalBabel) {


nit: space between if and (

eventualbuddha · 2017-12-30T17:17:13Z

README.md

+
+`babel-codemod` also supports non-standard/future language features that are not currently supported by the latest version of node.  It does this by leveraging `babel-preset-env` which loads the [latest babel plugins](https://github.com/babel/babel/tree/master/packages/babel-preset-env#support-all-plugins-in-babel-that-are-considered-latest).  This feature is on by default.
+
+This feature should support most use cases when writing plugins in advanced Javascript syntax. However, if you are writing plugins with syntax that is beyond "latest", or you would like to use your own set of plugins and presets, you can pass in the `--use-local-babel` switch in combination with a local `.babelrc` file that lists the presets/plugins you want applied to your plugin code.


nit: capitalize as “JavaScript”

eventualbuddha · 2017-12-30T17:17:40Z

README.md


+### Transpiling using typescript


nit: capitalize as “TypeScript”

@mcMickJuice

Adds `--[no-]transpile-plugins` option to control whether babel should be used to transpile plugins before loading them. This will allow plugins to be written using language features unsupported by the current version of nodejs. Typically and by default simply using `babel-preset-env` will transpile the language features that people will want to use. However, if required, support for loading a custom babel config is controlled by `--[no-]find-babel-config`. Thanks to @mcMickJuice for his work on the feature in #31.

eventualbuddha · 2018-01-03T14:11:46Z

Released in v1.3.0 with the final CLI options being --[no-]transpile-plugins (on by default) and --[no-]find-babel-config (off by default). Thanks @mcMickJuice!

mcMickJuice changed the title ~~Addeds option for running plugin through babel-preset-env pipeline~~ Adds option for running plugin through babel-preset-env pipeline Jun 25, 2017

eventualbuddha mentioned this pull request Dec 29, 2017

Add option to transpile TypeScript plugins directly #51

Closed

eventualbuddha requested changes Dec 30, 2017

View reviewed changes

eventualbuddha merged commit 4d7d95f into codemod-js:master Jan 3, 2018

eventualbuddha mentioned this pull request Jan 3, 2018

Add hint to add babel-register prior to using it #25

Closed

mcMickJuice mentioned this pull request Jan 6, 2018

Allow easier development of codemods #29

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds option for running plugin through babel-preset-env pipeline #31

Adds option for running plugin through babel-preset-env pipeline #31

mcMickJuice commented Jun 25, 2017

eventualbuddha commented Dec 22, 2017

mcMickJuice commented Dec 25, 2017

eventualbuddha commented Dec 25, 2017 •

edited

Loading

mcMickJuice commented Dec 28, 2017

eventualbuddha commented Dec 29, 2017

mcMickJuice commented Dec 29, 2017

mcMickJuice commented Dec 29, 2017

eventualbuddha commented Dec 29, 2017

mcMickJuice commented Dec 29, 2017

mcMickJuice commented Dec 29, 2017

eventualbuddha commented Dec 29, 2017

mcMickJuice commented Dec 29, 2017

eventualbuddha commented Dec 30, 2017

eventualbuddha commented Dec 30, 2017

mcMickJuice commented Dec 30, 2017

mcMickJuice commented Dec 30, 2017

mcMickJuice commented Dec 30, 2017

eventualbuddha left a comment

eventualbuddha Dec 30, 2017

eventualbuddha Dec 30, 2017

mcMickJuice Dec 31, 2017

eventualbuddha Dec 30, 2017

eventualbuddha Dec 30, 2017

eventualbuddha Dec 30, 2017

eventualbuddha Dec 30, 2017

eventualbuddha Dec 30, 2017

eventualbuddha Dec 30, 2017

mcMickJuice Dec 31, 2017

mcMickJuice Dec 31, 2017 •

edited

Loading

eventualbuddha Dec 31, 2017

eventualbuddha Dec 30, 2017

eventualbuddha Dec 30, 2017

eventualbuddha Dec 30, 2017

eventualbuddha commented Jan 3, 2018

		@@ -0,0 +1,7 @@
		import path from 'path'

		const i = path.join(__dirname, 'hello.js')


		`babel-codemod` also supports non-standard/future language features that are not currently supported by the latest version of node. It does this by leveraging `babel-preset-env` which loads the [latest babel plugins](https://github.com/babel/babel/tree/master/packages/babel-preset-env#support-all-plugins-in-babel-that-are-considered-latest). This feature is on by default.

		This feature should support most use cases when writing plugins in advanced Javascript syntax. However, if you are writing plugins with syntax that is beyond "latest", or you would like to use your own set of plugins and presets, you can pass in the `--use-local-babel` switch in combination with a local `.babelrc` file that lists the presets/plugins you want applied to your plugin code.

Adds option for running plugin through babel-preset-env pipeline #31

Adds option for running plugin through babel-preset-env pipeline #31

Conversation

mcMickJuice commented Jun 25, 2017

eventualbuddha commented Dec 22, 2017

mcMickJuice commented Dec 25, 2017

eventualbuddha commented Dec 25, 2017 • edited Loading

mcMickJuice commented Dec 28, 2017

eventualbuddha commented Dec 29, 2017

mcMickJuice commented Dec 29, 2017

mcMickJuice commented Dec 29, 2017

eventualbuddha commented Dec 29, 2017

mcMickJuice commented Dec 29, 2017

mcMickJuice commented Dec 29, 2017

eventualbuddha commented Dec 29, 2017

mcMickJuice commented Dec 29, 2017

eventualbuddha commented Dec 30, 2017

eventualbuddha commented Dec 30, 2017

mcMickJuice commented Dec 30, 2017

mcMickJuice commented Dec 30, 2017

mcMickJuice commented Dec 30, 2017

eventualbuddha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcMickJuice Dec 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eventualbuddha commented Jan 3, 2018

eventualbuddha commented Dec 25, 2017 •

edited

Loading

mcMickJuice Dec 31, 2017 •

edited

Loading