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

RFC 001: Improving language support #1159

Merged
merged 2 commits into from
Jan 13, 2017
Merged

Conversation

novemberborn
Copy link
Member

An RFC with a proposal for improving AVA's support for Babel, React and TypeScript projects.

We'll leave this open as a PR for a while to gather feedback. If accepted this incorporates / supersedes #707, #1082, #945, #631 and #1122.

@huan
Copy link

huan commented Dec 20, 2016

Great, hope we can add support to those languages soon.

As my view, if we could get rid of the limitation in ava-files, ie: do not restrict the file extension, just let users define them...

Then we can use ts-node or babel-node to run ava with the specific language support, without any more code design/modification, and everyone will be happy. :P

@novemberborn
Copy link
Member Author

As my view, if we could get rid of the limitation in ava-files, ie: do not restrict the file extension, just let users define them...

One of the patterns we support is one that matches a directory. AVA automatically expands that to find all .js files within. We couldn't do that if any extension is allowed, yet not having to add **/*.js to your patterns is quite nice.

Then we can use ts-node or babel-node to run ava with the specific language support, without any more code design/modification, and everyone will be happy. :P

While tempting, I think the performance cost will be too high. It'll probably also stop us from doing things like #789.

Copy link
Member

@sindresorhus sindresorhus left a comment

Choose a reason for hiding this comment

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

Thanks for writing this @novemberborn. It's well researched and argued.


The above assumes AVA is used with regular JavaScript projects that do not require compilation. Many users though already have a Babel pipeline in place and wish to use AVA without having to precompile their source files.

At it's simplest, setting `"babel": true` in the AVA configuration enables AVA's support for Babel projects. Test and helper files are compiled as per the above, but source files are now automatically compiled as well.
Copy link
Member

Choose a reason for hiding this comment

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

it's => its

AVA's handling of Babel projects can be further configured by passing an options object instead of `true`:

* `compileSources: true | false`: defaulting to `true`, determines whether sources are compiled.
* `extensions: "js" | ["js", "jsx", ...]`: defaulting to `"js"`, specifies the allowed file extensions. This expandn the default test file patterns.
Copy link
Member

Choose a reason for hiding this comment

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

expandn => expands


`sourceOptions` can be used to extend a shared Babel configuration so that the source files can be loaded in AVA tests. For instance users may [rely on webpack to resolve ES2015 module syntax at build time, but still need to apply `babel-plugin-transform-es2015-modules-commonjs` for sources to work in AVA][source options reason].

`sourceOptions` and `testOptions` may specify `ignore` and `only` values. These are only used to determine whether the file needs compilation. They do not impact test file selection or source watching.
Copy link
Member

Choose a reason for hiding this comment

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

It's not immediately clear that ignore and only refers to Babel options.


## Compilation

Based on this [proof of concept](https://github.com/avajs/ava/pull/1082) Babel compilation is moved into the test workers. If source files are to be compiled AVA will load its own require hook, rather than relying on `babel-core/register`.
Copy link
Member

Choose a reason for hiding this comment

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

👍


AVA's handling of Babel projects can be further configured by passing an options object instead of `true`:

* `compileSources: true | false`: defaulting to `true`, determines whether sources are compiled.
Copy link
Member

Choose a reason for hiding this comment

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

So does this mean that compilation of sources are now true by default without any config?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only if you set babel. So out of the box the behavior stays exactly the same as it is now (with helper compilation).

Copy link
Member

Choose a reason for hiding this comment

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

Good :)

Copy link
Contributor

@jfmengels jfmengels left a comment

Choose a reason for hiding this comment

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

Looks like a good plan to me.

I'm wondering though: For Babel compilation, AVA will probably always stick to the version it's shipped with, and not the version that was installed by the user in their project. I think that's fine as I don't think this has played us tricks as of yet.

But how will TS compilation be made? Will AVA have to ship with the TypeScript build tools? Or can we expect it as an optional peer dependency or something?

@sindresorhus
Copy link
Member

sindresorhus commented Jan 9, 2017

But how will TS compilation be made? Will AVA have to ship with the TypeScript build tools? Or can we expect it as an optional peer dependency or something?

We will not bundle TypeScript. Users will have to add TypeScript as a dependency. We'll throw a nice error if they forgot to add the dependency. (It will also not be a package.json peerDependency as it's not strictly a dependency, but more like an optional peer dependency).

@novemberborn
Copy link
Member Author

We might not even include the TypeScript support module (I think that's what I proposed here, but on mobile so don't feel like checking right now).

@vadimdemedes
Copy link
Contributor

Great work, @novemberborn! Thorough and to the point!

I wonder if we could come up with a better name for babelrc: true option, since it pulls config not only from .babelrc, but from babel prop in package.json.

@novemberborn
Copy link
Member Author

I wonder if we could come up with a better name for babelrc: true option, since it pulls config not only from .babelrc, but from babel prop in package.json.

Yea I hear ya. It's a (no longer documented, as far as I can tell) option in the transform APIs though. It refers both to .babelrc and the babel stanza in package.json. (In the CLI there is a negation flag: --no-babelrc.)

What's tricky is that the sourceOptions and testOptions objects are supposed to be passed to Babel as-is. I'll do some more research and get in touch with Babel people for what the preferred setting is.

@novemberborn
Copy link
Member Author

I'll do some more research and get in touch with Babel people for what the preferred setting is.

Though I haven't spoken with anybody, it seems pretty clear to me that it's babelrc. I've opened babel/babel#5101 and babel/website#1134 to clarify Babel's documentation.

Anything else or is this good to go?

@sindresorhus
Copy link
Member

:shipit:

@novemberborn novemberborn merged commit 076eb81 into master Jan 13, 2017
@novemberborn novemberborn deleted the improving-languange-support branch January 13, 2017 15:20
@novemberborn
Copy link
Member Author

I shall commence the project planning! 💥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants