8000 Fix the subtle bug with deprecation warning. by AlexKVal · Pull Request #608 · react-bootstrap/react-bootstrap · GitHub
[go: up one dir, main page]

Skip to content

Fix the subtle bug with deprecation warning. #608

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

Closed

Conversation

AlexKVal
Copy link
Member
@AlexKVal AlexKVal commented May 3, 2015

When deprecated Collaps_a_bleNav is imported
it also tags Collaps_i_bleNav element as deprecated, because
it is done by using pointers which points to the same object.

let CollapsableNav = CollapsibleNav;
CollapsableNav.__deprecated__ = true;

https://github.com/react-bootstrap/react-bootstrap/blob/92c57ef7ee/src/CollapsableNav.js#L5

The right and simplest way to make deprecation
in the case of component renaming CollapsableNav => CollapsibleNav
by using general for both components part - classSpecifications.

Like this:

// NewNameComponent.js
const classSpecificationsFor_NewNameComponent = { /* existing code */ };
const NewNameComponent =
  React.createClass( classSpecificationsFor_NewNameComponent );
export {classSpecificationsFor_NewNameComponent}; // for old component
export default NewNameComponent;

// OldNameComponent.js
import {classSpecificationsFor_NewNameComponent} from 'NewNameComponent';
const classSpecsFor_OldNameComponent =
  assign({}, classSpecificationsFor_NewNameComponent, {
  // here we add deprecation warning
  });
const OldNameComponent =
  React.createClass( classSpecsFor_OldNameComponent );
export default OldNameComponent;

Addresses #604
and #607

When deprecated `Collaps_a_bleNav` is imported
it also tags `Collaps_i_bleNav` element as deprecated, because
it is done by using pointers which points to the same object.

```js
let CollapsableNav = CollapsibleNav;
CollapsableNav.__deprecated__ = true;
```
https://github.com/react-bootstrap/react-bootstrap/blob/92c57ef7ee/src/CollapsableNav.js#L5

The right and simplest way to make deprecation
in the case of component renaming `CollapsableNav => CollapsibleNav`
by using general for both components part - `classSpecifications`.

Like this:

```js
// NewNameComponent.js
const classSpecificationsFor_NewNameComponent = { /* existing code */ };
const NewNameComponent =
  React.createClass( classSpecificationsFor_NewNameComponent );
export {classSpecificationsFor_NewNameComponent}; // for old component
export default NewNameComponent;

// OldNameComponent.js
import {classSpecificationsFor_NewNameComponent} from 'NewNameComponent';
const classSpecsFor_OldNameComponent =
  assign({}, classSpecificationsFor_NewNameComponent, {
  // here we add deprecation warning
  });
const OldNameComponent =
  React.createClass( classSpecsFor_OldNameComponent );
export default OldNameComponent;
```
@dozoisch
Copy link
Member
dozoisch commented May 3, 2015

we just need to make sure we deprecate the functions with collapsable in the name also!

@AlexKVal
Copy link
Member Author
AlexKVal commented May 3, 2015

my answer #604 (comment) ;)

@AlexKVal
Copy link
Member Author
AlexKVal commented May 3, 2015

About possible merge conflict with #603 (comment)

@dozoisch
Copy link
Member
dozoisch commented May 3, 2015

Well looks good to me, we should merge that asp I guess!

@react-bootstrap/collaborators If someone can take a look at it!

@mtscout6
Copy link
Member
mtscout6 commented May 4, 2015

LGTM

@mtscout6
Copy link
Member
mtscout6 commented May 4, 2015

@AlexKVal I'm wondering if you could rebase the changes for #609 onto this branch and have both PRs combined to a single PR, thus no merge conflicts.

94D8

@AlexKVal
Copy link
Member Author
AlexKVal commented May 4, 2015

This PR is rebased and combined with #609.

@AlexKVal AlexKVal closed this May 4, 2015
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.

3 participants
0