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

Allow configuration of built in source transformers. #58

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow configuration of built in source transformers. #58

wants to merge 1 commit into from

Conversation

m-orchard
Copy link

Hey,

We needed the ability to configure the istanbul transformer, specifically so we could tell it to use of ES6 modules when requiring (by passing through esModules: true).

I thought this might the best and most backwards compatible way of doing it - you can now specify configuration when registering a built-in transformer, and calling register for the same transformer after that will simply override the previous configuration. For the coffee transform, this configuration is passed straight into the compile call, and for istanbul this configuration is copied, then used to help detect if coverage is running (in case a custom coverageVariable is specified), then passed into the instrumentation call.

You can see I've not added any unit tests - I'll be honest, I wasn't sure the best way to test this. At least for the istanbul transformer, the instrumentation doesn't appear to be mocked, so there's not a nice way to check whether config is passed on. If you would like tests to be added I'd be happy to, with a little guidance.

Cheers!

@m-orchard
Copy link
Author

I see the failure, I'll try to get that sorted.

@m-orchard
Copy link
Author

Tests fixed. If you can offer any guidance on whether/how you want to test config being passed down, that would be great, but either way I will try to find time to write some.

@domenic
Copy link
Collaborator

domenic commented Apr 29, 2016

I don't have much time to review these patches these days, and it seems like the other maintainers don't either. In the past @felixge has been willing to add new collaborators for cases like this; would that work for you?

@felixge
Copy link
Owner

felixge commented May 1, 2016

I'd be happy to add @mickylad as a collaborator.

@m-orchard
Copy link
Author

Super slow response, sorry! I'm happy to be added as a contributor if you want - anything you need from me?

@felixge
Copy link
Owner

felixge commented Jul 29, 2016

@mickylad slow reply from me too :). I need your npm user name so I can add you as an owner.

@m-orchard
Copy link
Author

@felixge itsmickylad - just signed up! :)

@felixge
Copy link
Owner

felixge commented Sep 1, 2016

@mickylad added you on GH and NPM.

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

Successfully merging this pull request may close these issues.

3 participants