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

A possible way to provide customizable babel configs using .babelrc/env #448

Closed
jamestalmage opened this issue Jan 17, 2016 · 37 comments · Fixed by #708
Closed

A possible way to provide customizable babel configs using .babelrc/env #448

jamestalmage opened this issue Jan 17, 2016 · 37 comments · Fixed by #708
Assignees

Comments

@jamestalmage
Copy link
Contributor

Continued from:
#398 (comment)

My proposal is that we allow the following in .babelrc:

{
  "presets": ["es2015"], // base config
  "sourceMaps": "inline",
  "env": {
    "ava" : {  // if `env/ava` is present, we disable our custom config.
      "presets": ["stage-0"],
      "plugins": [] //maybe we automatically append power-assert for them?
    },
    "production" : {
     "sourceMaps": false
    }
  }
}

There are a couple questions that need to be answered in order to proceed. Specifically, can we change which env Babel uses on a per file basis? It might be as easy as setting process.env.BABEL_ENV before each call to babel.transform(..)

@billyjanitsch
Copy link
Contributor

My proposal is that we allow the following in .babelrc:

This is a great start! To keep my AVA and base configs identical, do I write "ava": {}? This is a little different from how the .babelrc env field works in general:

{
  "presets": ["es2015", "stage-0"], // base config
  "env": {
    "development" : {}, // commenting this out does nothing
    "ava" : {} // commenting this out does something!
  }
}

Do you have plans for the general case of a custom register hook, which may transpile imports using transpilers other than Babel? Is that what --no-babel is for?

For reference, here's the hook we're passing to Mocha (which is applied to test code via --compilers, as opposed to AVA's --require which targets tests' imports). The require extensions are used to nullify webpack's aliasing & loaders.

@jamestalmage
Copy link
Contributor Author

commenting this out does something!

Yeah - that is what I was thinking, but you raise a good point about that being an odd side-effect. If you have a different idea, this is the place to share.

Do you have plans for the general case of a custom register hook.

I think maybe --no-babel works in that scenario. My only concern is that leaves everyone else out in the cold when it comes to our caching and "transpile in the main thread" performance enhancements.

If anyone's got a list of alternative transpilers they wish they could use with AVA. I'd appreciate you sharing. Even better if you have in profiling information on those transpilers (require time via time-require and actual transpiling speed).

That said, this feels much more doable than providing performance enhancements for everyone.

@sindresorhus
Copy link
Member

I like the idea in general, but I'll need to think about how to handle the specifics.

maybe we automatically append power-assert for them?

👍 Would be too bad if people lost out on this one and can't think of any bad side-effect of doing it.

// @ariporad @JaKXz @MoOx @kentcdodds @sotojuan @SamVerschueren @kevva

@kentcdodds
Copy link
Contributor

"plugins": [] //maybe we automatically append power-assert for them?

My knee jerk reaction is to say no in favor of less magic ✨. I think that if you're going to override the default config, you should know what the default config is doing for you.

@sindresorhus
Copy link
Member

@kentcdodds I would generally agree, but think of power-assert more like an internal plugin. It doesn't affect the functionality of the code in any way. Babel is only one of many ways to add power-assert. We use Babel since it's already parsing the source, so we can take advantage of that. It's completely different from us choosing es2015 as a preset, which does affect the code.

@kentcdodds
Copy link
Contributor

That makes sense. I'm in favor 👍 Thanks for keeping me in the loop :D

@MoOx
Copy link

MoOx commented Jan 22, 2016

"plugins": [] //maybe we automatically append power-assert for them?

Make sense too. It's not like you are adding tons of transformations.

@ariporad
Copy link
Contributor

Hmm... Great idea @jamestalmage! However, This still seems a little weird to me. If we're already using the .babelrc, What if we just had a --custom-babel flag? (We could set the NODE_ENV to test so they could still customize their config).

@ghost
Copy link

ghost commented Feb 12, 2016

I note that one of XO's pluses is to pull config out of .foorc and into package.json where it can be more portable and visible. Since latest npm uses flattened module dirs now, it seems like it would be worth just having everything together in package.json in an "ava" field, and making ava responsible for having the deps it needs to handle the config it understands.

The "env" "ava" fields in the above .babelrc seems like a good structure for that. (I don't actually feel that strongly about where it goes, either--just that isolating the config to the limit of ava's concern seems prudent.)

@danielhusar
Copy link

Any news on this ? It will make testing react components easier :)

@vadimdemedes
Copy link
Contributor

@danielhusar No, not yet, can't give any ETA on this unfortunately :(

@develar
Copy link
Contributor

develar commented Feb 22, 2016

@vdemedes Am I right that discussion is over (initial proposal + add power-assert automatically)? And we need just implement it?

@sindresorhus
Copy link
Member

@develar There are still some unanswered questions:

There are a couple questions that need to be answered in order to proceed. Specifically, can we change which env Babel uses on a per file basis? It might be as easy as setting process.env.BABEL_ENV before each call to babel.transform(..)

@sindresorhus
Copy link
Member

@talexand You can put your Babel config in package.json under the babel field. We would of course support both.

@spudly
Copy link
Contributor

spudly commented Feb 23, 2016

Here's my two cents...

It seems to me that expecting the user to change their .babelrc or package.json/babel configs to modify the behavior of ava is backwards. Behavior specific to ava should be in a .avarc file or in package.json/ava.

For example, your .avarc file might contain the following:

{
  "babel": {
    "presets": [
      "es2015",
      "stage-0",
      "react"
    ]
  }
}

... and your package.json file would contain this...

{
  "babel": {
    "presets": [
      "es2015",
      "stage-2",
      "react"
    ]
  },
  "ava": {
    "babel": {
      "presets": [
        "es2015",
        "stage-0",
        "react"
      ]
    }
  },
}

This allows you to have separate babel configs for test and source files if you want.

We could also have some special keywords for the babel key.

  • default: Use the default config specified by ava (babel stage-2 I believe?). This is the same as omitting the key altogether.
  • inherit: Tests use the same babel config as source files (from .babelrc or package.json/babel). This allows you to share the same config without configuring it twice.

In package.json, it might look like this:

{
  "babel": {
    "presets": [
      "es2015",
      "stage-0",
      "react"
    ]
  },
  "ava": {
    "babel": "inherit",
  },
}

@kentcdodds
Copy link
Contributor

@spudly, I love the recommendation. I didn't really feel comfortable mucking around with babel's config. This seems much cleaner 👍

@sindresorhus
Copy link
Member

@spudly 👍 That's some very good cents. I like it.

@jamestalmage @vdemedes You good with this?

@vadimdemedes
Copy link
Contributor

I like it! Thanks @spudly!

@kentcdodds
Copy link
Contributor

Ideally, we would not write code to merge plugins/presets, and instead use Babel's internal 0ptionsManager#mergeOptions method.

This is why micro-libraries are reeeeally nice :-) Maybe someone (me?) could extract that logic into a well tested microlib that both tools use?

@spudly
Copy link
Contributor

spudly commented Feb 25, 2016

Added a pull request for all except the "extends" key. Let me know what you think.

@jamestalmage
Copy link
Contributor Author

This is why micro-libraries are reeeeally nice :-) Maybe someone (me?) could extract that logic into a well tested microlib that both tools use?

👍 - It might be best just to make a PR to Babel that extracts it into a microlib within the Babel monolith repo (that would likely be what the Babel team wants).

@sindresorhus
Copy link
Member

I think it should also be possible to disable Babel completely with: "babel": false, per #424.

@kentcdodds
Copy link
Contributor

"babel": false

👍

@sindresorhus
Copy link
Member

Reopening this as we still need to implement babel: false and the extends feature.

@sindresorhus sindresorhus reopened this Mar 7, 2016
@sindresorhus sindresorhus added enhancement new functionality and removed question labels Mar 7, 2016
@JaKXz
Copy link

JaKXz commented Mar 7, 2016

Love it so far, thanks guys!

@callumlocke
Copy link

"babel": false

👍

Is there any workaround technique that would have the same effect, until this ships? Would "babel": {} work?

@jamestalmage
Copy link
Contributor Author

Would "babel": {} work?

Sounds reasonable. Only one way to find out for sure ;)

@spudly
Copy link
Contributor

spudly commented Mar 8, 2016

Would "babel": {} work?

It'll still use babel, but it will not have any presets, so it won't transform anything.

@sindresorhus
Copy link
Member

New AVA version is out with customizable Babel config.

@jamestalmage
Copy link
Contributor Author

I think once #706 and #708 land, this issue is should be closed. The only issue not handled at that point is disabling babel. I think that is problematic, and will open a new issue to discuss.

@tomek-he-him
Copy link

Is it possible to disable babel via CLI? minimist translates --no-babel to babel: false – we could use that pattern here as well:

 minimist(['--no-babel'])
 { _: [], babel: false }

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

Successfully merging a pull request may close this issue.