Skip to content
This repository has been archived by the owner on May 17, 2019. It is now read-only.

Make react/enzyme dependency soft #473

Merged
merged 6 commits into from
Aug 16, 2018
Merged

Make react/enzyme dependency soft #473

merged 6 commits into from
Aug 16, 2018

Conversation

lhorie
Copy link
Contributor

@lhorie lhorie commented Aug 15, 2018

Currently fusion-cli depends on react and enzyme in order to be used at all. This was originally done to make it easier to test React, but the overall goal of fusion-cli is to be a CLI+compilation infrastructure that is agnostic of view libraries.

This PR allows fusion-cli to be installed and used without throwing react/enzyme-related errors, while still enabling them to be used if they're detected.

@old-fusion-bot
Copy link

Please add one of the following required labels:

  • breaking
  • feature
  • bugfix
  • docs
  • discussion
  • release
  • prerelease
  • greenkeeping

1 similar comment
@old-fusion-bot
Copy link

Please add one of the following required labels:

  • breaking
  • feature
  • bugfix
  • docs
  • discussion
  • release
  • prerelease
  • greenkeeping

KevinGrandon
KevinGrandon previously approved these changes Aug 15, 2018
Copy link
Contributor

@KevinGrandon KevinGrandon left a comment

Choose a reason for hiding this comment

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

Looking good to me so far, left a minor comment about removing the redbox-react dependency.

@@ -8,15 +8,7 @@

/* eslint-env node */

const React = require('react');
const RedBox = require('redbox-react').RedBoxError;
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're getting rid of this, we should remove the redbox-react dependency from package.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good catch!

@old-fusion-bot
Copy link

Please add one of the following required labels:

  • breaking
  • feature
  • bugfix
  • docs
  • discussion
  • release
  • prerelease
  • greenkeeping

@lhorie lhorie added the bugfix label Aug 15, 2018
KevinGrandon
KevinGrandon previously approved these changes Aug 15, 2018
Copy link
Contributor

@KevinGrandon KevinGrandon left a comment

Choose a reason for hiding this comment

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

LGTM assuming tests pass. Not seeing a very useful error stack in CI unfortunately =(

@lhorie lhorie merged commit 16a40bb into master Aug 16, 2018
@AlexMSmithCA AlexMSmithCA mentioned this pull request Aug 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants