Skip to content
This repository was archived by the owner on Jan 19, 2018. It is now read-only.

Problem with unknown options #2

Closed
MartinKolarik opened this issue Jan 30, 2015 · 11 comments
Closed

Problem with unknown options #2

MartinKolarik opened this issue Jan 30, 2015 · 11 comments
Labels

Comments

@MartinKolarik
Copy link

Since v3.0.0, 6to5 throws when you supply an unknown option, which means this module fails due to this line. If there's no other way to configure gobble, the key should probably be deleted before passing options to 6to5.

@Rich-Harris
Copy link
Contributor

Ah, shoot. Deleting the key is probably the way to go - the options object is cloned before the function is called (so that we can manipulate it in exactly this sort of way without consequence).

Though since the accept option is never used by plugins themselves but gobble's built-in map transformer, it might make sense to delete accept and ext from the default options object at startup as soon as the values have been read. Then plugins don't have to do anything.

The alternative (and arguably more 'correct') approach would be to structure the options like...

gobble( 'src' ).transform( '6to5', {
  to5Options: {
    blacklist: [ 'modules' ]
  }
});

...but I'm not really into that, it seems like overkill.

@MartinKolarik
Copy link
Author

it might make sense to delete accept and ext from the default options object at startup as soon as the values have been read. Then plugins don't have to do anything.

That makes sense. It's probably the best solution.

@sebmck
Copy link

sebmck commented Jan 30, 2015

Damn, I tried to catch all of these when I was updating all the plugins to 6to5 3.0.0. I'll add accept as a valid option as I've done with some of the other plugins that used custom options.

sebmck added a commit to babel/babel that referenced this issue Jan 30, 2015
@sjmeverett
Copy link

@sebmck what about ext?

@sebmck
Copy link

sebmck commented Mar 5, 2015

@stewartml What about it?

@sjmeverett
Copy link

Sorry for the brevity. Using ext I can change the extension from say .es6 to .js; however babel complains about it. I can fix this by either adding it as a valid option to babbel, or by adding a line to delete the option in gobble-babel.

I wonder though: surely it's the job of the plugins not to send bad options rather than babel to special case any options that might come in from plugins, or have I got the wrong end of the stick?

@sebmck
Copy link

sebmck commented Mar 5, 2015

@stewartml Yeah, gobble-babel should be removing all invalid options before passing it on to babel.

@sebmck
Copy link

sebmck commented Mar 5, 2015

The only reason I whitelisted some in the first place was to avoid having to go and update all the plugins as I'm not familiar at all with a large majority of the libraries that there are plugins for.

@sjmeverett
Copy link

I see, that's fair enough. I could submit a patch for gobble-babel to delete accept and ext, or perhaps @Rich-Harris would know better all the keys that should be deleted before passing to babel.

@Rich-Harris
Copy link
Contributor

Thanks guys, you're right about it not being babel's responsibility - I've chosen to fix this in gobble itself by treating accept and ext options with map transforms as 'special' (which they are) - gobblejs/gobble#36. That way other plugins won't encounter this problem.

I agree, it makes sense to allow .es6, though personally I just use .js since a) it's just JavaScript and b) it really should be .es2015 now :)

Have published gobble 0.7.5 and gobble-babel 4.0.1 with this change.

@sjmeverett
Copy link

Thanks for this. You're right, I would just use .js myself too, but it's in a project where I'll avoid some potential issues if I make it obvious that there's some extra translation happening.

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

Successfully merging a pull request may close this issue.

4 participants