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

Request: support for babel options as an object #46

Closed
MoOx opened this issue Mar 26, 2015 · 23 comments
Closed

Request: support for babel options as an object #46

MoOx opened this issue Mar 26, 2015 · 23 comments

Comments

@MoOx
Copy link
Contributor

MoOx commented Mar 26, 2015

I think webpack provide an API to accept any kind of option (query string or object) via the webpack instance .option[key] but I am not sure.
That being said, that could be great to define options like this

{
  //...
  babel: {
    experimental: true,
    playground: true,
    optional: [
      "runtime",
      "spec.protoToAssign",
    ],
    loose: "all",
  },
}

(And because query strings are so ugly)

Example of loader I made that use this simple way to read option:

https://github.com/MoOx/eslint-loader/blob/defe509dbb98e31d781dae35fa544c350730c411/index.js#L73
https://github.com/cssnext/cssnext-loader/blob/c7b79f5281a5e63adca933fc4ec434f965e686e3/index.js#L22

@Couto
Copy link
Member

Couto commented Mar 26, 2015

I'm not really sure if I understood your point correctly, but webpack accepts json in the querystring: https://github.com/webpack/loader-utils

so I guess you could pass the object by 'json.stringifying' it into the query.

@MoOx
Copy link
Contributor Author

MoOx commented Mar 26, 2015

i've done that. But it will much cleaner to support global webpack object for option too. I guess most people use babel-loader once.
Stringify on the front to unstringify on the back is not optimized.

@Couto
Copy link
Member

Couto commented Mar 26, 2015

While I can understand the point of legibility and convenience (and I do agree that query strings are far from perfect) I get the feeling that this is something that should be supported by webpack itself, instead of having plugins "extending" the functionality with no major gains.

I honestly think that this issue should be issued to webpack instead of the loaders, since that way every loader would benefit from the change, and users wouldn't have to deal with different ways of passing options.

Anyway I wouldn't mind to listen the opinion of @sebmck since he also maintains the loader.

@sebmck
Copy link
Contributor

sebmck commented Mar 26, 2015

@Couto I don't really have an opinion since I don't personally use webpack so I have no idea what's most convenient for people so feel free to take the lead on this one. 😄

@Couto
Copy link
Member

Couto commented Mar 26, 2015

I tried to search around the issues in webpack and I found #482 I think this solves the problem.

I'm closing this. Since I prefer to be consistent and predictable with other loaders, if the way of passing options changes, or something else becomes the norm, I don't mind at all to adapt.

@Couto Couto closed this as completed Mar 26, 2015
@Couto
Copy link
Member

Couto commented Mar 26, 2015

Just to make it clear :)

I think you should open an issue in webpack's issue tracker, suggesting your way of passing options.
You'll have my support on that. The current way of passing options is awkward to say the least. I just don't think that this should be fixed at the loader level.

@MoOx
Copy link
Contributor Author

MoOx commented Mar 26, 2015

The way I suggest option is already supported by webpack, it's just another webpack API to use in the loader itself. I can only advice you to double check the links I mentioned earlier in the issue and check API of those loaders well, you should get it.

Example

// webpack.config.js
module.exports = {
  //...
  module: {
    loaders: [
    {
      test:   /\.js$/,
      loader: "babel-loader!a&b"
    }
    ],
  },
  babel: {
    a: false,
    b: false,
    c: true,
  },
}
// babel-loader
module.exports = function(input) {
  // you will just need to update this lines in babel-loader
  // this merge global option + loader specific option
  // making you able to have global babel option + specific loader option
  // (in case you use babel-loader differently on some files according to your regex test)
  var options = Object.assign({}, this.options.babel, loaderUtils.parseQuery(this.query))
  // options are {a: true, b: true, c: true}
}

@Couto
Copy link
Member

Couto commented Mar 26, 2015

I checked the links, I understood the implementation.

Can you give me a link to some issue or docs showing that webpack "officially" supports that way of reading options?

I just need something from webpack stating that I can do this, instead of relying on undocumented options that later can disappear.

@MoOx
Copy link
Contributor Author

MoOx commented Mar 26, 2015

I think this is it http://webpack.github.io/docs/loaders.html#options

@Couto
Copy link
Member

Couto commented Mar 26, 2015

Works for me then

Are you up for a PR? Otherwise I’ll only be able to do it during the weekend.

On 26 Mar 2015, at 18:06, Maxime Thirouin notifications@github.com wrote:

I think this is it http://webpack.github.io/docs/loaders.html#options


Reply to this email directly or view it on GitHub.

@MoOx
Copy link
Contributor Author

MoOx commented Mar 26, 2015

I will try to do that tomorrow morning.

@Couto Couto reopened this Mar 26, 2015
@MoOx
Copy link
Contributor Author

MoOx commented Mar 27, 2015

With ddos attacks I can't fork this morning :(
I will try later if you didn't do it before me :)
FYI, I implemented the same thing we discuss in eslint-loader (but can't show you right now since push on github isn't working for now :D)

@Couto
Copy link
Member

Couto commented Mar 28, 2015

Worry not :)

I'll be merging the branch with develop later.

Couto added a commit that referenced this issue Mar 28, 2015
* feature/options:
  Closes #46. Add support for babel global options.
  Remove parsing of booleans.
@vjpr
Copy link

vjpr commented May 7, 2015

+1

@Couto
Copy link
Member

Couto commented May 8, 2015

@vjpr that's already implemented in the develop branch I'm just trying to find some free time to make a release.

@sebmck
Copy link
Contributor

sebmck commented May 9, 2015

You can already provide webpack loader query options via an object:

module.exports = {
  module: {
    loaders: [
      {
        test: /\.js$/
        exclude: /node_modules/,
        loader: 'babel-loader',
        query: {
          // your babel options here
        }
      }
    ]
  }
};

Adding yet another way to specify options is probably going to be awkward.

@Couto
Copy link
Member

Couto commented May 9, 2015

Uh! that is new, and nice :)
I think the main advantage was to have a global option object, but this makes everything easier :)

@MoOx
Copy link
Contributor Author

MoOx commented May 9, 2015

Hum, so I guess we can close this issue ?

@dashed
Copy link
Contributor

dashed commented May 15, 2015

Probably a compromise would be to just update README to show this in addition to the shorthand (e.g. loader: 'babel-loader?optional=runtime').

@Couto Couto closed this as completed in c6928d1 May 15, 2015
Couto added a commit that referenced this issue May 15, 2015
* release/5.1.0:
  Update README.md
  Update dependencies.
  Version bump
  Update README.md
  Update README.md
  Fix package.json version
  CS. Remove peerDependencies completely.
  Fix CS. Update README.md.
  Update test options to support babel 5
  Update dependencies.
  [WIP] Add test to output babel sourcemap
  Fix merge from develop.
  [WIP] Start work with sourcemaps.
  Closes #46. Add support for babel global options.
  Remove parsing of booleans.
  Update depedencies. Add files to .gitignore
  Add coverage to help with testing.
  [WIP] Add cache idenfier as option.
  [WIP] Move cache related functions to lib.
  Move cache related functions to own file.
@deini
Copy link

deini commented Mar 8, 2016

Sorry to bring back this issue to life but could we consider this again? I think I have a legitimate use case.

I'm trying to use Babel's getModuleId option which expects a function. However we can't pass functions through query.

Please correct me if I'm wrong (and I hope I am)

This is also related to #118

@MoOx
Copy link
Contributor Author

MoOx commented Mar 8, 2016

You can. Query accept a real object #46 (comment)

@deini
Copy link

deini commented Mar 8, 2016

Which AFAIK gets stringified, and loses the function.

@deini
Copy link

deini commented Mar 8, 2016

Letting this here just in case somebody else tumbles to this.

Just went trough the code and figured I can do this by just passing babel: {} to the webpack config.

https://github.com/babel/babel-loader/blob/master/index.js#L36 I don't think its documented.

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

No branches or pull requests

6 participants