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

WIP - allow custom pre-processors #292

Closed

Conversation

jamestalmage
Copy link
Contributor

This adds custom preprocessor support.

No automated test yet. Run the demo like this:

./cli.js --compilers=coffee:cofee-script/register ./test/fixture/custom-processor.coffee

Currently it runs our own Babel transform on the coffee-script transpiled code. This allows the use of generators, async/await, etc. without users having to manually configure Babel. I am not sure this is the right strategy. While it provides an easy path to use ES2015 goodness in your coffee-script tests, there will likely be scenarios where users want to specify their own Babel setup, and just use our runner.

  1. Should we include a flag to disable our Babel hooks?
  2. Should automatically disable our Babel hook when we see --compilers= or --require=?
  3. If we do 2, should we provide a way to reenable?

@@ -215,7 +221,7 @@ function handlePaths(files) {
})
.then(flatten)
.filter(function (file) {
return path.extname(file) === '.js' && path.basename(file)[0] !== '_';
return path.extname(file) === '.coffee' || path.extname(file) === '.js' && path.basename(file)[0] !== '_';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: That was a quick hack I meant to fix

@novemberborn
Copy link
Member

Rather than depending on coffeescript is it more illustrative to code up a simple .txt "compiler"?

@novemberborn
Copy link
Member

I am not sure this is the right strategy. While it provides an easy path to use ES2015 goodness in your coffee-script tests, there will likely be scenarios where users want to specify their own Babel setup, and just use our runner.

This would work though wouldn't it? I bring my own Babel set up, and make sure my require hook is loaded first. Then AVA compiles power assertions and async/await on top of that. I don't have to worry about configuring either of those and AVA doesn't have to care how I write my code.

@jamestalmage
Copy link
Contributor Author

Rather than depending on coffeescript is it more illustrative to code up a simple .txt "compiler"?

Sure - I will probably steel from whatever simplified transforms you end up adding to the nyc test suite. 😜

This would work though wouldn't it?

I'm not sure, but I really doubt it. The issue blocking our Babel 6 adoption is actually related to Babel running a particular transform twice. (It transforms all typeof statements, but the generated code contains a typeof statement, which gets transformed again).

Perhaps (with Babel 6), we could fallback to only running the power-assert transform, instead of the full es2015 preset.

@jamestalmage
Copy link
Contributor Author

The more I think about this, we really can't run the power-assert transform after regenerator or asyncToGenerator transforms. power-assert needs to see the original AST - it uses it to pass a copy of the source to it's renderers.

t.is(await foo, await bar);

becomes (very approximately):

t.is(t._expr(t._capt(await foo), {source: 't.is(await foo, await bar'}), ...

applying after the asyncToGenerator transform, the source passed to power-assert would end up scrambled.

t.is(ctx.$0$0, ctx.$0$1)

I am beginning to wonder if it would not be best to just adopt a "take it or leave it" approach to the transform setup we provide. If users want to provide their own transform they need to provide a complete one, and we just don't touch it. We can provide documentation (and possibly convenience methods) for including the power-assert transform.

At a minimum, I think we need to provide a way for them to tell us to skip our transform via a flag in the CLI.

// @sindresorhus @vdemedes

@novemberborn
Copy link
Member

Perhaps (with Babel 6), we could fallback to only running the power-assert transform, instead of the full es2015 preset.

That's what I was thinking, yes.

The more I think about this, we really can't run the power-assert transform after regenerator or asyncToGenerator transforms. power-assert needs to see the original AST - it uses it to pass a copy of the source to it's renderers.

Yes that sounds like a show-stopper for this. Too bad! (Unless we revert that line using a source map… but that gets too complicated.)

I am beginning to wonder if it would not be best to just adopt a "take it or leave it" approach to the transform setup we provide. If users want to provide their own transform they need to provide a complete one, and we just don't touch it. We can provide documentation (and possibly convenience methods) for including the power-assert transform.

At a minimum, I think we need to provide a way for them to tell us to skip our transform via a flag in the CLI.

👍

This adds custom preprocessor support.

No automated test yet. Run the demo like this:

    ./cli.js --compilers=coffee:cofee-script/register ./test/fixture/custom-processor.coffee

Currently this runs Babel on the coffeescript transpiled code.
 This allows the use of generators, async/await, etc. without
 users having to manually configure Babel.
 I am not sure this is the right strategy.
 While automatically configuring Babel would be cool,
 I think it will likely end up being better to just disable
 our internal Babel support as soon as they pass a
 `--compilers` or `--require` command. Basically, once you've
 done that, you've taken transpilation on yourself.
 An alternate possibility would be a flag to manually
 disable our own Babel processing.
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.

2 participants