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

Convert to plugin #81

Open
jmm opened this issue Feb 4, 2015 · 20 comments
Open

Convert to plugin #81

jmm opened this issue Feb 4, 2015 · 20 comments

Comments

@jmm
Copy link
Collaborator

jmm commented Feb 4, 2015

This is pretty much what BC-breaking conversion to a plugin would look like right now (as discussed in, and based on, #80).

exports = module.exports = function plugin (b, opts) {
  if (opts.basedir === undefined) opts.basedir = b._options.basedir;
  b.transform(es6ify(opts));
};

Latest: bc-breaking-plugin HEAD

@thlorenz
Copy link
Owner

thlorenz commented Feb 4, 2015

Any way we could make it not breaking? Just asking cause in proxiquireify it was possible somehow.

@jmm
Copy link
Collaborator Author

jmm commented Feb 4, 2015

I think so, I was just going by @domenic's comment where it sounded like he favored converting it to a plugin exclusively (BC-breaking), but I may have misunderstood.

@thlorenz
Copy link
Owner

thlorenz commented Feb 4, 2015

Well it'll be a major release, so we could break things.
People just need to upgrade their browserify. It has had plugins now for a good while.
So go ahead break things :)

@domenic
Copy link
Collaborator

domenic commented Feb 4, 2015

I'd like to also include the runtime automatically, when we do a plugin. (I guess we can have an option to turn it off but you really shouldn't be using Traceur without the runtime.)

@jmm
Copy link
Collaborator Author

jmm commented Feb 4, 2015

but you really shouldn't be using Traceur without the runtime.

What if you just want to transpile ES6 module syntax?

@domenic
Copy link
Collaborator

domenic commented Feb 4, 2015

Then you should probably be using a dedicated transpiler for that (e.g. es6-module-transpiler or whatever), cuz at any moment Traceur could update their transpilation to depend on the runtime (and in fact at least one of the module output formats does depend on it).

@jmm
Copy link
Collaborator Author

jmm commented Feb 4, 2015

Well it'll be a major release, so we could break things.
People just need to upgrade their browserify. It has had plugins now for a good while.
So go ahead break things :)

Ok. What versions of browserify do you want to support?

@thlorenz
Copy link
Owner

thlorenz commented Feb 4, 2015

@jmm the last 5 majors should be sufficient and plugins have been around that long.

@jmm
Copy link
Collaborator Author

jmm commented Feb 4, 2015

Then you should probably be using a dedicated transpiler for that (e.g. es6-module-transpiler or whatever), cuz at any moment Traceur could update their transpilation to depend on the runtime (and in fact at least one of the module output formats does depend on it).

Ok, thanks.

So plugin with runtime included by default and option to opt-out?

@thlorenz
Copy link
Owner

thlorenz commented Feb 4, 2015

@domenic talking about that, I'm wondering what es6ify at this point provides that modules like 6to5 don't?

@thlorenz
Copy link
Owner

thlorenz commented Feb 4, 2015

So plugin with runtime included by default and option to opt-out?

Sounds good to me.

@jmm
Copy link
Collaborator Author

jmm commented Feb 4, 2015

the last 5 majors should be sufficient and plugins have been around that long.

Ok, thanks. What's a strategy for testing that?

@domenic
Copy link
Collaborator

domenic commented Feb 4, 2015

@jmm yeah that sounds good to me.

@thlorenz well Traceur is in general better than 6to5, in a large part because it's written by people who are deep into the spec. 6to5 fixes spec compliance issues as fast as I can file them, which is to their credit, but I would rather use Traceur.

@domenic
Copy link
Collaborator

domenic commented Feb 4, 2015

Ok, thanks. What's a strategy for testing that?

I usually just test the latest and wait for people to file bugs on anything else :P.

@jmm
Copy link
Collaborator Author

jmm commented Feb 4, 2015

I usually just test the latest and wait for people to file bugs on anything else :P.

Ok, I'll go with that for now.

Since we're breaking BC there's no reason for traceurOverrides to be an export now, right? It can just be an option to the plugin?

@domenic
Copy link
Collaborator

domenic commented Feb 4, 2015

Agreed. Might also be good to remove compileFile.

@jmm
Copy link
Collaborator Author

jmm commented Feb 4, 2015

Agreed. Might also be good to remove compileFile.

Up to you guys. That didn't have use cases that are still relevant?

@jmm
Copy link
Collaborator Author

jmm commented Feb 5, 2015

Re: the discussion in #25 about being able to do this...

browserify('./entry')
  // Includes runtime by default
  .plugin(es6ify)

...vs having to do this to get the runtime executed first...

browserify()
  .plugin(es6ify)
  .add('./entry')

It looks like @guzart was dealing with that by somehow prepending the runtime to the bundle and adjusting source maps accordingly. I haven't nailed it down 100%, just skimming. I experimented with a different approach, which there's a prototype of in jmm@33ec7d0. I haven't tested it extensively or tried to analyze all the possible ramifications, it's just an experiment so far. Even if it seemed like it wouldn't screw anything up I don't know if you guys want to rely on browserify internals like that.

@jmm
Copy link
Collaborator Author

jmm commented Feb 5, 2015

In order to run the transform tests as currently configured you still need access to the function that's registered with b.transform(func) inside the plugin (what es6ify.configure() previously returned). Since browserify doesn't do anything with plugin return values, I set it up so that calling the plugin function (the main export) will return the transform function. So in the tests where it had:

something.pipe(es6ify.configure(opts))

It's this instead:

something.pipe(es6ify(browserify(), opts))

For that scenario it means you either have to test the first arg to es6ify() to see if it's a browserify instance before calling methods like add() on it, or pass a browserify instance (or something imitating it). So far I took the approach of just passing a browserify instance.

@jmm
Copy link
Collaborator Author

jmm commented Mar 30, 2015

Re: plugin vs transform, there'll still need to be a transform for package.json, right?

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

3 participants