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

Temporarily bundle jQuery and SUI JS #327

Merged
merged 1 commit into from
Jul 10, 2016
Merged

Conversation

levithomason
Copy link
Member

@levithomason levithomason commented Jul 7, 2016

Fixes #326. This PR address the issue of two copies of jQuery being loaded. One on the the window via script tag, and another via the webpack bundle.

To simplify things for users for now, we simply bundle jQuery and SUI JS with Stardust. This is only temporary as we are moving quickly to remove jQuery in #247.

"lodash": "^4.6.1"
"jquery": "^3.1.0",
"lodash": "^4.6.1",
"semantic-ui-css": "^2.2.1"
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll be so glad the day I delete these two lines.

@levithomason levithomason force-pushed the feature/handle-jquery branch 2 times, most recently from d2ab76e to 2779b7e Compare July 8, 2016 01:17
$: 'jquery',
jQuery: 'jquery',
'window.jQuery': 'jquery',
}),
Copy link
Member Author

Choose a reason for hiding this comment

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

We don't expose jQuery on the window up front. We attach it to the window ourselves after require()ing it. We do this so we can throw an error if the user has already loaded it on the window before we load.

@levithomason levithomason force-pushed the feature/handle-jquery branch 2 times, most recently from 4a655cb to 0c0f018 Compare July 10, 2016 15:30
@@ -160,7 +155,8 @@ webpackConfig.module.loaders = [{
// ----------------------------------------
// For faster builds in dev, rely on prebuilt libraries
// Local modules can still be enabled (ie for offline development)
if (argv.localModules) {
// in TEST we need local modules because karma uses a different index.html (no CDNs)
if (__TEST__ || argv.localModules) {
Copy link
Member Author

Choose a reason for hiding this comment

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

In tests we use local modules, there are no CDNs.

@levithomason levithomason force-pushed the feature/handle-jquery branch from 0c0f018 to 8289447 Compare July 10, 2016 15:32
@levithomason levithomason merged commit ba96811 into master Jul 10, 2016
@levithomason levithomason deleted the feature/handle-jquery branch July 10, 2016 15:42
jhchill666 pushed a commit to jhchill666/stardust that referenced this pull request Jul 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant