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

Fix: Import Babel config from package.json and export in babel.config.js to fix Jest tests in custom plugins that have a package.json #4782

Conversation

dancastellon
Copy link
Contributor

@dancastellon dancastellon commented Oct 25, 2018

Resolves #4781
Impact: minor
Type: bugfix

Issue

Babel 7 doesn't transpile files in sub-directories that have a package.json when Babel is configured through package.json or .babelrc. This causes Jest test failures in custom plugins that have a package.json. It isn't an issue with non-test files because those are imported through the main Reaction app (see /server/plugins.js and client/plugins.js). Babel does transpile these files when it is configured through the new babel.config.js. Meteor currently only loads Babel config through .babelrc or package.json, though.

Solution

Meteor only reads the babel config from .babelrc or package.json. So with just babel.config.js, the Meteor app fails but the Jest tests pass. With just package.json, the Jest tests in custom plugins fail but the app runs.

In order to support Babel transpiling of Jest tests, we load the babel config defined in package.json and export it in the new babel.config.js.

Breaking changes

None

Testing

  1. Create a folder inside imports/plugins/custom
  2. In the folder, run npm init and create an empty package.json
  3. Create a file named log.js with console.log("test"); in it
  4. Create a log.test.js file with the following content:
import "./log";
test("Simple test", () => { expect(true).toBe(true); });
  1. Back in the reaction root dir, run meteor npm run test:unit
  2. Confirm "Simple test" passes

….js so that tests in custom plugins w/ package.json files work
@dancastellon dancastellon changed the base branch from master to release-2.0.0-rc.6 October 25, 2018 19:47
@spencern
Copy link
Contributor

@dancastellon What were the reasons you decided to read in the existing package.json babel section rather than just moving the babel config? Is it dynamically configured right now?

@dancastellon
Copy link
Contributor Author

@spencern Meteor only reads the babel config from .babelrc or package.json. So with just babel.config.js, the Meteor app fails but the Jest tests pass. With just package.json, the Jest tests in custom plugins fail but the app runs.

Copy link
Contributor

@spencern spencern left a comment

Choose a reason for hiding this comment

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

Would like to note the Meteor dependency on keeping the babel configuration within Package.json in the code. Looks good otherwise.

module.exports = function (api) {
api.cache(false);

const file = fs.readFileSync("./package.json");
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to add a note here mentioning that this is required because as you mentioned

Meteor only reads the babel config from .babelrc or package.json. So with just babel.config.js, the Meteor app fails but the Jest tests pass. With just package.json, the Jest tests in custom plugins fail but the app runs.

So that we're aware of why we did this and if Meteor changes or our dependency on Meteor changes we can change this in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spencern Comment added

/**
* Meteor only reads the babel config from .babelrc or package.json. So with just babel.config.js,
* the Meteor app fails but the Jest tests pass. With just package.json, the Jest tests in custom plugins
* fail but the app runs.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@spencern spencern merged commit 84fef6b into release-2.0.0-rc.6 Oct 26, 2018
@spencern spencern deleted the fix-4781-dancastellon-jest-tests-plugins-w-package-json branch October 26, 2018 14:28
@spencern spencern mentioned this pull request Nov 6, 2018
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