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

Add a build step to transpile ES6 -> ES5 #9

Closed
wants to merge 1 commit into from
Closed

Add a build step to transpile ES6 -> ES5 #9

wants to merge 1 commit into from

Conversation

rreusser
Copy link

@rreusser rreusser commented Aug 18, 2016

See: #8
cc: @nattelog

Long story short, I was unable to use this module out of the box because my project didn't otherwise have any use for a ES6 build step. I could add one, but I generally think it's nicer for a casual dependency not to force it. I apologize for the change in line endings (I can try to undo that if you prefer cleaner git history), but the only changes are:

  • adds build directory lib/ to .gitignore
  • adds source directory src/ to .npmignore
  • changes entry point to lib/index
  • adds a prepublish step that builds and tests
  • adds necessary dev dependencies to get everything to work out of the box

I know this is an opinionated PR so you're absolutely free to disregard this, no hard feelings; this is just me voicing a preference without just +1'ing the issue. 😄

Thanks for the great module!

@rreusser rreusser mentioned this pull request Aug 18, 2016
@rreusser
Copy link
Author

rreusser commented Aug 18, 2016

Update: added babelify + es2015 to my demo (normal fresnel reflection!—used it to smooth out the control interactions 😄). I still lean toward a build step, but I'll leave the ball in your court on whether to accept/reject.

@Philmod
Copy link
Owner

Philmod commented Aug 18, 2016

Hey,

If you change the entrypoint to compiled lib/index.js, will that still work if you import that npm module in a node.js application?

I don't believe this module should be compiled in any form. The application that uses it should take care of that.

Your demo looks really cool!

@rreusser
Copy link
Author

rreusser commented Aug 18, 2016

That's fair. 👍

The specific point of failure for me is that uglifyjs fails when it encounters class, so that the only option is transpiling. Which isn't a big deal. Everyone transpiles these days; just wasn't convinced it should be mandatory. I'm using a custom budo dev server, so just had to add some transpiling configuration. I can see an argument for both sides, but I'm perfectly happy. Thanks!

@rreusser rreusser closed this Aug 18, 2016
@rreusser
Copy link
Author

rreusser commented Aug 18, 2016

Oh, sorry. I didn't answer your question. Yes, if the entry point is lib/index.js and that's in the node module but not the git repo, it'll work from node.js. (I've used that approach to avoid either the node module or the git repo having two different versions of the same code. The node module has the compiled version and the git repo has the source; no overlap)

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.

2 participants