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

[xy-chart] use babel instead of webpack to support deep imports #83

Closed
wants to merge 1 commit into from

Conversation

williaster
Copy link
Owner

🏆 Enhancements

  • This PR does a bit of refactoring in @data-ui/xy-chart so it's built with babel instead of webpack in order to support deep imports in addition to package-level imports.

    This should allow users to minimize bundle size by importing only the components they need instead of the entire package. Once webpack 4 comes out this will be moot, and we'll add the sideEffects flag to the package.json

// deep imports
import XYChart from '@data-ui/xy-chart/build/chart/XYChart';

// or, package-level
import { XYChart } from '@data-ui/xy-chart';

fixes #49
cc @lencioni

@coveralls
Copy link

coveralls commented Dec 8, 2017

Coverage Status

Coverage remained the same at 84.457% when pulling 8114223 on chris--babelify-xy-chart into a2eae66 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.0%) to 81.481% when pulling 8114223 on chris--babelify-xy-chart into a2eae66 on master.

@lencioni
Copy link

lencioni commented Dec 8, 2017

If you don't have any side effects, I don't think there's any reason to wait adding that value to your package.json so it will be ready whenever people get on webpack 4.

Also, you probably should add an ESM build that avoids compiling your imports/exports into CommonJS, so tree-shaking can actually do its job (CommonJS deoptimizes tree-shaking, since tree-shaking is enabled by ES module semantics).

@williaster
Copy link
Owner Author

Abandoning this in favor of #111

@williaster williaster closed this Jul 27, 2018
@williaster williaster deleted the chris--babelify-xy-chart branch July 27, 2018 22:05
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.

use absolute imports for vx components, update builds to support absolute imports for data-ui components
3 participants