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

babel 6 upgrade #9702

Merged
merged 15 commits into from
Feb 9, 2017
Merged

Conversation

coverslide
Copy link
Contributor

@coverslide coverslide commented Jan 3, 2017

Note this is mostly piggybacking off of @spalger 's work on PR #7010 in reference to issue #5822

I rebased that branch onto master and made the necessary changes to get it working properly.

Some notes on the changes:

  • Use babel plugins and presets instead of a list of specific settings
  • The "export default" problem has been fixed by adding the "add-module-exports" plugin. This allows us to mix require() statements with es6 modules without having to modify anything.
  • The "es2015-node" preset solves the issue of separating node-specific transforms and browser-specific transforms. It also feature-detects based on the running version of node.
  • The "class-properties" plugin seems to need loading before the presets so that was added to the plugins list.

This PR has been linted and tested from the latest master.

@elasticmachine
Copy link
Contributor

Can one of the admins verify this patch?

@coverslide coverslide force-pushed the implement/babel6-update-2 branch 3 times, most recently from d1e8cf8 to d9a0628 Compare January 3, 2017 10:09
@spalger spalger self-assigned this Jan 3, 2017
@spalger
Copy link
Contributor

spalger commented Jan 3, 2017

Thanks @coverslide! I'll take a look at this soon

@spalger spalger added the v5.2.0 label Jan 3, 2017
@coverslide coverslide force-pushed the implement/babel6-update-2 branch 2 times, most recently from ed1bf96 to b5bbc34 Compare January 4, 2017 16:59
@coverslide
Copy link
Contributor Author

@spalger rebased to fix a conflict

@spalger
Copy link
Contributor

spalger commented Jan 7, 2017

jenkins, test this

@coverslide coverslide force-pushed the implement/babel6-update-2 branch 4 times, most recently from 2ec807e to b04c97b Compare January 12, 2017 07:36
@coverslide
Copy link
Contributor Author

@spalger made a few fixes, I ran locally and fails 1 selenium test, but that same test was failing on master so it may just be my machine, could we re-run jenkins on this?

@epixa epixa removed the v5.2.0 label Jan 17, 2017
@coverslide coverslide force-pushed the implement/babel6-update-2 branch from 3b3e518 to 017e731 Compare January 21, 2017 06:52
@coverslide
Copy link
Contributor Author

coverslide commented Jan 21, 2017

@spalger I found the issue, and now all tests pass for me! Could we check this again? Sorry, hopefully third time is the charm!

It looks like the way we require ui/notifier from kbn_chrome.js still has the require(...).default bug, but I don't see the issue anywhere else.

@coverslide coverslide force-pushed the implement/babel6-update-2 branch 3 times, most recently from 9732765 to 4544a20 Compare January 25, 2017 21:49
@coverslide
Copy link
Contributor Author

@spalger fixed a merge conflict, any chance we can re-check this?

@coverslide coverslide force-pushed the implement/babel6-update-2 branch from 4544a20 to 0eebcb5 Compare January 27, 2017 18:23
@coverslide
Copy link
Contributor Author

@epixa I see the tests passed here: https://kibana-ci.elastic.co/job/elastic-kibana-pull-request/3668/ but the kibana-ci check didn't update. I think I pushed up the latest update after the check had already started. There will be no more pushes.

@epixa
Copy link
Contributor

epixa commented Jan 27, 2017

jenkins, test this

@coverslide
Copy link
Contributor Author

@spalger @epixa Any movement on this?

@spalger spalger added the Team:Operations Team label for Operations Team label Feb 6, 2017
@coverslide coverslide force-pushed the implement/babel6-update-2 branch from bf49ac6 to 94c2054 Compare February 6, 2017 23:48
@coverslide
Copy link
Contributor Author

@spalger awesome. I noticed one of the scripts did not run properly when tested. Looking into it.

@spalger
Copy link
Contributor

spalger commented Feb 7, 2017

jenkins, test this

@spalger spalger requested review from tylersmalley and jbudz February 7, 2017 21:31
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@spalger spalger removed the request for review from tylersmalley February 7, 2017 21:31
}

if (!env.WEBPACK_BABEL_CACHE_DIR) {
env.WEBPACK_BABEL_CACHE_DIR = fromRoot('optimize/.webpack.babelcache');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file doesn't seem to be generating, if it's supposed to be can we add it here to play nice with package permissions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I enabled it in dev, but it's disabled in the distributable because it creates a directory and we won't be able to ensure all of it's files are chowned

@jbudz
Copy link
Member

jbudz commented Feb 9, 2017

LGTM for core, can we make sure plugins are updated too?

@spalger spalger merged commit 8b4c052 into elastic:master Feb 9, 2017
spalger pushed a commit to spalger/kibana that referenced this pull request Feb 9, 2017
* [npm] upgrade babel

The upgrade to babel 6 requires an upgrade to all of the associated modules, which meant that a few other things changed at the same time. The most notable is the way that we handle our babel-options, which is now done with an npm module and includes using the babel-loader's "presets" query string param.

This meant changes to the babel_options.js module and extending it to help setting up the "babel-register" module, which was previously copy-pasted in several places.

* [mtodules] upgrade to support babel6 module semantics

* [eslint] fix lint errors

* [babel] ignoer massive fixture files

* [cli/errors] use Object.setPrototypeOf since subclassing Error is broken

* [babel] Upgrading core babel libraries

[babel] Use WIP babel-6-fix branch of babel-preset-kibana

* Fix broken test

* [babel] Reverse unnecessary module.exports changes

* Fix notifier

* Use babel presets and plugins directly

* [babel/options] resolve preset/plugins paths for better plugin compatibility

* [babel/options] use babel-preset-env for correct node settings

* [babel] cache babel compilation in webpack like we thought we were
@spalger spalger mentioned this pull request Feb 9, 2017
@spalger spalger added the v5.4.0 label Feb 9, 2017
spalger added a commit that referenced this pull request Feb 10, 2017
* [npm] upgrade babel

The upgrade to babel 6 requires an upgrade to all of the associated modules, which meant that a few other things changed at the same time. The most notable is the way that we handle our babel-options, which is now done with an npm module and includes using the babel-loader's "presets" query string param.

This meant changes to the babel_options.js module and extending it to help setting up the "babel-register" module, which was previously copy-pasted in several places.

* [mtodules] upgrade to support babel6 module semantics

* [eslint] fix lint errors

* [babel] ignoer massive fixture files

* [cli/errors] use Object.setPrototypeOf since subclassing Error is broken

* [babel] Upgrading core babel libraries

[babel] Use WIP babel-6-fix branch of babel-preset-kibana

* Fix broken test

* [babel] Reverse unnecessary module.exports changes

* Fix notifier

* Use babel presets and plugins directly

* [babel/options] resolve preset/plugins paths for better plugin compatibility

* [babel/options] use babel-preset-env for correct node settings

* [babel] cache babel compilation in webpack like we thought we were
@spalger spalger mentioned this pull request Feb 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Operations Team label for Operations Team v5.4.0 v6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants