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: remove Slider from the global export #932

Merged

Conversation

jmfrancois
Copy link
Contributor

What is the problem this PR is trying to solve?

Each time we add a new component with a dependency it becomes hard for every one to update.
Think to add this deps into the project.
It's also embed in the build because all components are exposed by the global index.js.

This is the case with rc-slider.

What is the chosen solution to this problem?

Let's remove Slider from index.js (breaking change)

This will be done for every components so we will decrease our bundle size.
The main idea is to import component from it's folder when it's possible.
Here we have update forms package to do so.

Please check if the PR fulfills these requirements

  • The PR commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Related design / discussions / pages (not in jira), if any, are all linked or available in the PR

[ ] This PR introduces a breaking change

@@ -3,7 +3,7 @@ import React from 'react';

import RJSForm from 'react-jsonschema-form/lib/index';

import { Action } from '@talend/react-components';
import Action from '@talend/react-components/lib/Actions/Action';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this one is weird, we should put Action at the root (I think)

Copy link
Contributor

@jsomsanith-tlnd jsomsanith-tlnd left a comment

Choose a reason for hiding this comment

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

Can you add an entry in BREAKING_CHANGES.log please ?

@build-travis-ci
Copy link
Collaborator

:octocat: Demo is available here

@build-travis-ci
Copy link
Collaborator

:octocat: Demo is available here

@jsomsanith-tlnd jsomsanith-tlnd merged commit 73b605a into master Dec 26, 2017
@jsomsanith-tlnd jsomsanith-tlnd deleted the jmfrancois/fix/forms/remove-the-need-of-slider branch December 26, 2017 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants