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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed babel preset and use latest node as target when running dev #2484

Merged
merged 1 commit into from Feb 19, 2018

Conversation

@wubzz
Copy link
Member

@wubzz wubzz commented Feb 18, 2018

馃檶 Thanks for using Babel: we recommend using babel-preset-env now: please read babeljs.io/env to update!

Replaced babel-preset-es2015 with babel-preset-env (https://babeljs.io/env/)

I also changed so that running npm run dev targets latest node so that debugging code in lib during development is easier as not everything gets transpiled, such as classes etc.

@wubzz
Copy link
Member Author

@wubzz wubzz commented Feb 18, 2018

@elhigu Are you fine with this dev change?

I find it very obnoxious having to debug the transpiled code in development, for instance when writing tests.

@elhigu
Copy link
Member

@elhigu elhigu commented Feb 19, 2018

I would love to drop babel all together after node 4 gets to end of line.

Node 6 already supports destructuring, ... -operator and default parameters which are the most used more advanced javascript features used in knex.

@elhigu
elhigu approved these changes Feb 19, 2018
Copy link
Member

@elhigu elhigu left a comment

looks good.

@wubzz
Copy link
Member Author

@wubzz wubzz commented Feb 19, 2018

@elhigu I'd have loved to drop babel as well, but unfortunately it might not be possible even when Node 4 is dropped, since many people still use knex in browser environments for query building purposes.

Unless we at that point compromise and leave the transpiling up to the end user, of cousre. That's an option.

@wubzz wubzz merged commit a0abdd0 into knex:master Feb 19, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@elhigu
Copy link
Member

@elhigu elhigu commented Feb 19, 2018

AFAIK knex already needs dome trickery when one tries to use it as query builder on web page, like patching some missing requires etc.

I would like to know how many people are actually using it on webpage. One way to figure it out would be to drop support and if there are many people angry about it we could add build for web which omits pool / connection stuff and would work out of box when included to webpage.

Need to think more about it after may :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants