Skip to content

Export "webpack helper" function to aid usage in downstream Webpack projects#473

Merged
waxlamp merged 28 commits intomasterfrom
webpack-helper
Feb 13, 2017
Merged

Export "webpack helper" function to aid usage in downstream Webpack projects#473
waxlamp merged 28 commits intomasterfrom
webpack-helper

Conversation

@waxlamp
Copy link
Member

@waxlamp waxlamp commented Dec 23, 2016

  • Publishes a function in candela/webpack.js to aid in adding loaders and other peculiar-to-Candela configuration to a downstream project's Webpack configuration in a black-box way
  • Moves source files from src/candela into the top level of the repo - this makes it easier for downstreams to include Candela sources in their projects:
import LineChart from 'candela/components/LineChart';

instead of

import LineChart from 'candela/src/candela/components/LineChart';
  • Updates the LineUp and UpSet dependencies to versions that don't cause problems when Candela is used downstream this way

TODO:

  • Generate new LineUp image test after verifying that the component is behaving properly
  • Adjust .npmignore to restrict what gets published to NPM
  • Update documentation

@waxlamp waxlamp changed the title Export "webpack helper" function to aid usage in downstream Webpack projects [WIP] Export "webpack helper" function to aid usage in downstream Webpack projects Dec 23, 2016
@waxlamp
Copy link
Member Author

waxlamp commented Dec 23, 2016

@mgrauer, we can use this branch to try out importing Candela piecemeal into TrackerDash.

@mgrauer
Copy link
Contributor

mgrauer commented Dec 24, 2016

Is this ready for me to try, or would it be better to debug it together?

I've got in my downstream

import 'nvd3/build/nv.d3.min.css';
import './main.styl';
import candela from 'candela';
  
window.TrackerDash = candela.components.TrackerDash;

should that change?

☃️

@waxlamp
Copy link
Member Author

waxlamp commented Dec 27, 2016

You should be able to try it yourself, though I assume we will need to follow up with some debugging (either together or just me). Here are the steps to try it out:

  1. Change your Candela dependency in package.json to "git+ssh://git@github.com/kitware/candela.git#webpack-helper".

  2. Remove node_modules/candela.

  3. Run npm prune if you like.

  4. Run npm install.

  5. Change the Candela import statement to

    import TrackerDash from 'candela/components/TrackerDash';

    and the next line to

    window.TrackerDash = TrackerDash;
  6. Rebuild the project.

  7. Call/email/text/telegraph Roni with the list of errors.

@jeffbaumes
Copy link
Contributor

Interesting. So this removes src and puts the components dir at the top level?

@waxlamp
Copy link
Member Author

waxlamp commented Jan 3, 2017

Yes, and also the VisComponents and util directories (essentially, everything "top-level"). It makes for a bit of an uglier codebase, which is the cost of a smoother downstream experience.

This also removes font-awesome from the dependencies list; it doesn't seem to be necessary.
…nfusion between import styles

i.e. the following line

````javascript
import candela from 'candela';
````

will import the UMD bundle, while

````javascript
import candela from 'candela/candela';
````

will import the top-level file directly (allowing downstream Webpack to build it
into something probably similar to the original bundle).
@waxlamp waxlamp changed the title [WIP] Export "webpack helper" function to aid usage in downstream Webpack projects Export "webpack helper" function to aid usage in downstream Webpack projects Jan 9, 2017
@waxlamp
Copy link
Member Author

waxlamp commented Jan 9, 2017

This is ready for review.

@waxlamp waxlamp requested a review from jeffbaumes January 9, 2017 20:20
@codecov-io
Copy link

codecov-io commented Jan 10, 2017

Codecov Report

Merging #473 into master will increase coverage by 0.28%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #473      +/-   ##
==========================================
+ Coverage   40.38%   40.66%   +0.28%     
==========================================
  Files          38       36       -2     
  Lines        1263     1237      -26     
==========================================
- Hits          510      503       -7     
+ Misses        753      734      -19
Impacted Files Coverage Δ
components/TrackerDash/StatusBarWidget.js 23.52% <ø> (ø)
components/TrackerDash/colors.js 100% <ø> (ø)
components/OnSet/index.js 13.63% <ø> (ø)
components/ScatterPlot/index.js 80% <ø> (ø)
util/index.js 73.33% <ø> (ø)
components/index.js 100% <ø> (ø)
components/Histogram/index.js 80% <ø> (ø)
VisComponent/mixin/InitSize.js 50% <ø> (ø)
util/vega/index.js 70.56% <ø> (ø)
components/Geo/index.js 20% <ø> (ø)
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d1aa12...ad864f2. Read the comment docs.

@jeffbaumes
Copy link
Contributor

When building from scratch just now, the Resize and Serialization examples give a Uncaught SyntaxError: Unexpected token import. Also, OnSet has an error saying $ is not found.

@jeffbaumes
Copy link
Contributor

I pushed a fix for the Resize and Serialization examples. It was because candela.js was not going through Babel. I'm ok with the OnSet error getting offloaded to another issue that we solve separately.

Copy link
Contributor

@jeffbaumes jeffbaumes left a comment

Choose a reason for hiding this comment

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

This has been out there a while and its working to use this branch externally with a prototype Girder plugin, so LGTM.

@waxlamp
Copy link
Member Author

waxlamp commented Feb 8, 2017

Thanks Jeff. I notice that in addition to the undefined-$ problem, the resize examples don't actually resize. We can address that separately as well.

@waxlamp waxlamp merged commit 2339d4b into master Feb 13, 2017
@waxlamp waxlamp deleted the webpack-helper branch February 13, 2017 18:14
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.

4 participants