Skip to content

chore: fix addBabelPresets doc and test babel fns#98

Merged
arackaf merged 5 commits into
arackaf:masterfrom
with-heart:test/97-add-babel-presets
Jun 24, 2019
Merged

chore: fix addBabelPresets doc and test babel fns#98
arackaf merged 5 commits into
arackaf:masterfrom
with-heart:test/97-add-babel-presets

Conversation

@with-heart

@with-heart with-heart commented Jun 15, 2019

Copy link
Copy Markdown
Collaborator

Closes #97

I investigated #97 and discovered that addBabelPresets works similarly to addBabelPlugins in that it spreads its arguments and then maps them. This means that passing an array of presets instead of a flat list of arguments will result in an array nested one level deeper than expected.

Correct usage example:

module.exports = override(
  ...addBabelPresets(
    [
      "@babel/env",
      {
        targets: {
          browsers: ["> 1%", "last 2 versions"]
        },
        modules: "commonjs"
      }
    ],
    "@babel/preset-flow",
    "@babel/preset-react"
  )
);

Changes

  • added a note in readme.md to not pass an array to addBabelPresets
  • removed the surrounding array from the addBabelPresets arguments in the readme.md code example
  • installed jest
  • added tests for addBabelPreset and addBabelPresets to guarantee their usage matches this PR
  • added tests for the rest of the babel functions
  • replaced npm test script with jest call
  • added coverage/ to .gitignore for jest --coverage usage

This change installs `jest@^24` and replaces the `test` script with a
call to `jest`.
This change adds tests for `addBabelPreset` and `addBabelPresets`. This
adds a test case for a reported bug with `addBabelPresets` that was
caused by incorrect documentation of the api.
This change adds a note not to pass an array of presets to
`addBabelPresets` and removes the passed array in the code example.
@with-heart with-heart changed the title refactor: test and fix addBabelPresets doc chore: fix addBabelPresets doc and test babel fns Jun 15, 2019
@arackaf

arackaf commented Jun 15, 2019

Copy link
Copy Markdown
Owner

@FezVrasta you have a second to give this a quick test run? I haven’t even used CRA in upwards of a year at this point, so I’m barely qualified to accept PR’s to my own project :)

@arackaf

arackaf commented Jun 24, 2019

Copy link
Copy Markdown
Owner

@with-heart ok I'm just gonna merge these changes. Hey, would you be interested in being a contributor to this project? I'll give you write access to the repo, and publish rights on npm, if you want.

@arackaf arackaf merged commit 52955c4 into arackaf:master Jun 24, 2019
@arackaf

arackaf commented Jun 24, 2019

Copy link
Copy Markdown
Owner

Published in 0.2.14

@with-heart

with-heart commented Jun 25, 2019

Copy link
Copy Markdown
Collaborator Author

@arackaf Awesome! Also, I'd love that, thank you :) I've had a few ideas brewing that I've been meaning to propose after fleshing out the test suite a little more.

@with-heart with-heart deleted the test/97-add-babel-presets branch June 25, 2019 02:36
@arackaf

arackaf commented Jun 25, 2019

Copy link
Copy Markdown
Owner

@with-heart what's your npm username?

@with-heart

Copy link
Copy Markdown
Collaborator Author

@arackaf ~with-heart

@arackaf

arackaf commented Jun 25, 2019

Copy link
Copy Markdown
Owner

Is this you? Let me know if not!!!

image

@with-heart

Copy link
Copy Markdown
Collaborator Author

@arackaf it is :)

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.

...addBabelPresets doest work

2 participants