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

Drop Node 4 support; Avoid eagerly requiring plugins and presets #223

Merged
merged 4 commits into from May 25, 2018

Conversation

Projects
None yet
4 participants
@mikrostew
Copy link
Contributor

mikrostew commented May 23, 2018

This is a working version of #218, to enable parallel transpilation with the default configuration.

This adds the use of require.resolve to enable multiple independent versions of ember-cli-babel to coexist, and fixes the tests to account for the changes.

@mikrostew

This comment has been minimized.

Copy link
Contributor Author

mikrostew commented May 23, 2018

The tests are hanging because the builds are running in parallel, in worker processes that continue to run after the build completes. So mocha is hanging waiting for those processes to exit and they never do.

Since this worker pool is shared between all broccoli-babel-transpilers of the same version, shutting down the workers immediately after a build would possibly create a lot of churn.

We could shut down the worker processes after they have been idle for some time. I had thought of doing that before, to mitigate ember-cli/ember-cli#7379.

@stefanpenner @rwjblue thoughts?

@stefanpenner

This comment has been minimized.

Copy link
Member

stefanpenner commented May 23, 2018

I think we should not rely on them aging out in tests

Rather we should likely explicitly kill them for now from the tests (and age out in addition seems fine) but I would hate for my tests to hang (slowing down my own productivity)

The correct solution would be to shutdown workers once the plugin or build pipeline instance that spawned them is cleanedup/destroyed. That may not be practical today (if it is we should doit) So instead, we can likely just manually kill them when the tests shutdown.

@stefanpenner

This comment has been minimized.

Copy link
Member

stefanpenner commented May 24, 2018

Spoke with @rwjblue, we agreed that destroying of workers should happen in plugin.cleanup(). @mikrostew can you do that?

@stefanpenner

This comment has been minimized.

Copy link
Member

stefanpenner commented May 25, 2018

Some funky windows test failures, I am:

  • going to bump the timeout slightly
  • consider dropping node 4 testing, since its EOL.
try bumping timeout, seeing some timeouts on appveyor in the following
tests "includes class transform when targets require plugin: "

@stefanpenner stefanpenner force-pushed the mikrostew:bring-back-parallel branch from 07d1b98 to f852120 May 25, 2018

@stefanpenner stefanpenner merged commit 6ba2ed1 into babel:master May 25, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@stefanpenner

This comment has been minimized.

Copy link
Member

stefanpenner commented May 25, 2018

released as v6.13.0 🎉

@Turbo87

This comment has been minimized.

Copy link
Contributor

Turbo87 commented May 28, 2018

consider dropping node 4 testing, since its EOL.

☝️ that was a breaking change!

@Turbo87 Turbo87 added the breaking label May 28, 2018

@Turbo87 Turbo87 changed the title Avoid eagerly requiring plugins and presets Drop Node 4 support; Avoid eagerly requiring plugins and presets May 28, 2018

@rwjblue

This comment has been minimized.

Copy link
Member

rwjblue commented May 28, 2018

😭 we need to revert e33466e at least (and add 10 manually)...

@mikrostew mikrostew deleted the mikrostew:bring-back-parallel branch May 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment