Refactoring, env, closing a bunch of issues

[denysdovhan]

Aims to fix #52 and #41.

[shvaikalesh]

One more thing: eslint-config-brunch if you are up to. Thanks.

[loganfsmyth]

Generally I think the Babel team as a whole has mixed feelings about enabling plugins/presets by default like this FYI, but it seems like that has already been released as a feature, right?

Given that, the currently-used approach (in babel-loader and babel-register) to resolving the config is the following:

const OptionManager = require('babel-core').OptionManager;

const opts = new OptionManager().init({
    // whatever options you'd pass to Babel normally.
});

// Check both, not just `plugins`.
const hasConfig = 
  (opts.plugins && opts.plugins.length > 0) ||
  (opts.presets && opts.presets.length > 0);

if (!hasConfig) {

}

It's an ugly API but please do not try to hardcode this. If we fix bugs or add new config methods like the newly-landed for 7.x .babelrc.js, your code will just break again.

[denysdovhan]

@shvaikalesh reasonable notice from Babel's Slack:

Actually, this PR could be easily resolved by removing these lines:

-if (!opts.presets) {
-     opts.presets = ['latest'];
-     // ['env', opts.env]
-}

Unfortunately, it would break backward compat.

[paulmillr]

I don't understand why "having defaults" is a bad idea. Brunch is all about simplicity and no bullshit. We don't want users to write configs when the program can think for them instead.

[denysdovhan]

@paulmillr okay. Anyway, I'm gonna find a better solutions. Will update my PR soon.

@loganfsmyth don't we run the risk using some kind of internal APIs? Probably, you would like to change Babel's internal API and then we will find ourselves in the situation when our users get broken builds.

[loganfsmyth]

don't we run the risk using some kind of internal APIs? Probably, you would like to change Babel's internal API and then we will find ourselves in the situation when our users get broken builds.

I'm not sure what you mean? The point of the OptionManager API is to get just the data you're asking about.

[shvaikalesh]

Agreed with Paul here. We should have cool defaults, like babel-preset-env. Related: https://github.com/babel/babel-brunch/issues/54

[denysdovhan]

I'm not sure what you mean?

@loganfsmyth OptionManager is an internal API, isn't it? That means you wouldn't warn us when you change that API.

[loganfsmyth]

It definitely won't break in 6.x and I'm planning to specifically leave a dummy class of export class OptionManager { init(opts){ ...call internal api... } } in 7.x for backward-compatibility because we want the transition from 6.x to 7.x to be as seamless as possible for people.

It's used by babel-register and babel-loader, which are both independent of babel-core, so any change to it at this point would also break those, which is a no-go for us.

[denysdovhan]

@loganfsmyth it seems strange but, if project uses such .babelrc:

{
  "presets": [
    "latest"
  ],
  "plugins": [
    "transform-class-properties"
  ]
}

This console.log call will produce:

{ filename: 'app/initialize.js',
  filenameRelative: undefined,
  inputSourceMap: undefined,
  env: {},
  mode: undefined,
  retainLines: false,
  highlightCode: true,
  suppressDeprecationMessages: false,
  presets: [],
  plugins:
   [ [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], [Object] ] ],
  ignore: [],
  only: undefined,
  code: true,
  metadata: true,
  ast: true,
  extends: undefined,
  comments: true,
  shouldPrintComment: undefined,
  wrapPluginVisitorMethod: undefined,
  compact: 'auto',
  minified: false,
  sourceMap: false,
  sourceMaps: false,
  sourceMapTarget: undefined,
  sourceFileName: 'app/initialize.js',
  sourceRoot: undefined,
  babelrc: true,
  sourceType: 'module',
  auxiliaryCommentBefore: undefined,
  auxiliaryCommentAfter: undefined,
  resolveModuleSource: undefined,
  getModuleId: undefined,
  moduleRoot: undefined,
  moduleIds: false,
  moduleId: undefined,
  passPerPreset: false,
  parserOpts: false,
  generatorOpts: false }

Why presets fields remains empty? As I understand, plugins from babel-preset-latest are listed in plugins.

[loganfsmyth]

The output of init is the full resolved config of babel, so in the majority of cases, all of the presets will have already been flattened into a simple array of plugins. There will be presets in the list still for people making use of passPerPreset: true in their config/presets though, because then there are multiple groups of plugins to be executed.

[denysdovhan]

@loganfsmyth by the way, you're saying OptionManager is used in babel-loader, but, as I see, there is a resolve-rc.js file for that usage.

[loganfsmyth]

Huh, it appears you're right, maybe it was an older version that I was thinking about. It's definitely used in babel-register then. I'm surprised babel-loader is doing that, and I disagree with that for the same reason I disagreed above :P

[denysdovhan]

@loganfsmyth unfortunately, passPerPreset didn't change the situation:

{ filename: 'app/initialize.js',
  filenameRelative: undefined,
  inputSourceMap: undefined,
  env: {},
  mode: undefined,
  retainLines: false,
  highlightCode: true,
  suppressDeprecationMessages: false,
  presets: [],
  plugins:
   [ [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], [Object] ] ],
  ignore: [],
  only: undefined,
  code: true,
  metadata: true,
  ast: true,
  extends: undefined,
  comments: true,
  shouldPrintComment: undefined,
  wrapPluginVisitorMethod: undefined,
  compact: 'auto',
  minified: false,
  sourceMap: false,
  sourceMaps: false,
  sourceMapTarget: undefined,
  sourceFileName: 'app/initialize.js',
  sourceRoot: undefined,
  babelrc: true,
  sourceType: 'module',
  auxiliaryCommentBefore: undefined,
  auxiliaryCommentAfter: undefined,
  resolveModuleSource: undefined,
  getModuleId: undefined,
  moduleRoot: undefined,
  moduleIds: false,
  moduleId: undefined,
  passPerPreset: true, // passed as true
  parserOpts: false,
  generatorOpts: false }

presets remains empty.

[loganfsmyth]

If I had to guess, that's because passPerPreset is kind of garbage in 6.x and latest runs straight into those edge cases. If you switch it to env or es2015 it should show separate.

[denysdovhan]

@loganfsmyth nope, this behavior remains the same for latest as well as for env and es2015. I've already tried.

[loganfsmyth]

> var OptionManager = require('babel-core').OptionManager;
undefined
> new OptionManager().init({filename: 'tmp.js', passPerPreset: true, presets: ['babel-preset-latest']})
{ sourceType: 'module',
  babelrc: true,
  filename: 'tmp.js',
  code: true,
  metadata: true,
  ast: true,
  comments: true,
  compact: 'auto',
  highlightCode: true,
  plugins: 
   [ [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], undefined ],
     [ [Object], undefined ],
     [ [Object], [Object] ],
     [ [Object], [Object] ] ],
  passPerPreset: true,
  presets: [ { presets: [Object] } ] }
> new OptionManager().init({filename: 'tmp.js', passPerPreset: true, presets: ['es2015']})
{ sourceType: 'module',
  babelrc: true,
  filename: 'tmp.js',
  code: true,
  metadata: true,
  ast: true,
  comments: true,
  compact: 'auto',
  highlightCode: true,
  passPerPreset: true,
  presets: [ { plugins: [Object] } ] }

[denysdovhan]

@loganfsmyth you're passing preset to init(), but I don't. I use OptionManager for detecting presets field in .babelrc, etc. Unfortunately, it expands presets as set of plugins. Take a look at: https://github.com/denysdovhan/babel-brunch-issue-52

[loganfsmyth]

Are you hoping to detect presets specifically in the config, while ignoring plugins? Do you default to latest even if the user has provided a list of plugins? I may have missed that detail. I was assuming it only defaulted to latest when no configuration at all was provided to Babel, so I thought the presence of a non-empty plugins or non-empty presets array from OptionManager would be enough.

I think I was misunderstanding what behavior you were hoping for. The preset grouping would happen for .babelrc too, but passPerPreset would have to be inside the .babelrc itself, since it is a flag for the user, not a flag for config lookup like you're trying to do. I mentioned it as an indication that you needed to check both on .presets and .plugins when deciding to add latest, not because you should be the one checking it.

[denysdovhan]

@loganfsmyth so, is there any other way to check if there are defined presets besides dirty checking with some kind of resolve-rs.js?

[loganfsmyth]

so, is there any other way to check if there are defined presets

There are not. I'm a little frustrated by this correct goal honestly because it breaks the consistency of the config system. From a config standpoint there should be zero difference between

{
  plugins: ['x'],
}

and

{
    presets: [
        {plugins: ['x']},
    ]
}

and by specifically trying to detect presets you're essentially monkeypatching the config system with nonstandard behavior.

We want meant to be able to bundle up their config into presets if they want to. You either have plugins or you don't. Babel's transformation system doesn't even really know presets exist.

[loganfsmyth]

There are also plenty of cases where people don't want to use an existing preset and just drop in the plugins they want, and this would try to add latest on top of that with no way for the user to turn it off.

[denysdovhan]

Now babel-brunch uses env, detects UglifyJS and sets corresponding target if it's needed. At this point, it should be enough to fix #52, #54 and #41.

@shvaikalesh @paulmillr what do you think about targets options for babel-brunch proposed by @yavorsky? (https://github.com/babel/babel-brunch/pull/53#discussion_r107199291)

[denysdovhan]

Done, I think. Can be merged.

[millerized]

Hi @denysdovhan. I would offer to help contribute and get this past the finish line, but it looks like you are almost there.

I am looking forward to using brunch with env support – great work! 🍻