Skip to content

Remove unused isCompositeComponentElement re-export#1023

Closed
aweary wants to merge 1 commit into
enzymejs:masterfrom
aweary:remove-is-composite-component-element
Closed

Remove unused isCompositeComponentElement re-export#1023
aweary wants to merge 1 commit into
enzymejs:masterfrom
aweary:remove-is-composite-component-element

Conversation

@aweary

@aweary aweary commented Jul 6, 2017

Copy link
Copy Markdown
Collaborator

This method is being deprecated soon react/react#10096

We don't actually use it at all internally, so this change just removes the unused import/export.

The rewrite doesn't include this import/export so this may be a moot PR, but I'm not sure if we intend to make any more minor releases before it's accepted.

@ljharb

ljharb commented Jul 7, 2017

Copy link
Copy Markdown
Member

Removing an export from any file is semver-major. Instead, let's include this change in the rewrite branch, so that the rewrite can be v3.

@ljharb ljharb added the semver: major Breaking changes label Jul 7, 2017
@aweary

aweary commented Jul 7, 2017

Copy link
Copy Markdown
Collaborator Author

Semver only dictates major releases for breaking changes to the public API, which should be well defined.

For this system to work, you first need to declare a public API. This may consist of documentation or be enforced by the code itself. Regardless, it is important that this API be clear and precise.

Enzyme has a pretty clear public API defined by the documentation, so I would argue this isn't a breaking change since react-compat exports are not defined as part of the public API (and never should be).

@aweary

aweary commented Jul 7, 2017

Copy link
Copy Markdown
Collaborator Author

Either way, we can close this if we want to consider it semver major since the rewrite branch doesn't contain this import/export anymore.

@ljharb

ljharb commented Jul 7, 2017

Copy link
Copy Markdown
Member

Our public API is defined as "everything that can possibly be reachable or observed", regardless of what's in documentation, which is also the way I think all projects should define it.

@aweary

aweary commented Jul 7, 2017

Copy link
Copy Markdown
Collaborator Author

I think that's an ill-defined way to describe a public API, but if that's the standard for Enzyme so be it. In that case, this PR is unnecessary since the same change will be made in #1007

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

Labels

semver: major Breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants