Skip to content
This repository has been archived by the owner on Oct 15, 2021. It is now read-only.

Feature/es6 modules #86

Closed
wants to merge 38 commits into from
Closed

Feature/es6 modules #86

wants to merge 38 commits into from

Conversation

reneviering
Copy link

Hey,

i made a POC using ES6-Modules. The current status is:

  • browserify integration
  • babelify as a plugin of browserify for transpiling
  • Make use of externals to move vendors out of the bundle
  • React is integrated as a playground

We should talk about the need of the dependency configuration which is used to add frontend-dependencies in a more generic way. Maybe it's obsolete because of the use of browserify.

@mischah
Copy link
Member

mischah commented Mar 18, 2016

Whoop. You the man

We should talk about the need of the dependency configuration which is used to add frontend-dependencies in a more generic way. Maybe it's obsolete because of the use of browserify.

I guess it is. See http://blog.npmjs.org/post/112064849860/using-jquery-plugins-with-npm

Let’s use this branch to work together.

We should drop a line over here before we address an issue. To avoid working simultaneously on the same thing :octocat:

@reneviering reneviering changed the title Feature/es6 modules [WIP] Feature/es6 modules Apr 5, 2016
@reneviering
Copy link
Author

Now there's no need to add script-tags to use jquery and bootstrap. I require bootstrap and jquery inside the module where it's needed.

But what about CSS? Is there a need to include CSS from libs the same way (i.e. with https://github.com/rotundasoftware/parcelify)? Otherwise, one would still touch the Gruntfile for adding CSS.

@mischah
Copy link
Member

mischah commented Apr 5, 2016

+1 for parcelify.

BUT. If maintainers don’t set the style property in their package.json we have to take care of telling parcelify which CSS files from a depnendency it has to include in the CSS bundle. This can be defined in our package.json which feels better than editing the gruntfile.

I don’t know if there are other options to automate bundling of vendor CSS.
Maybe @krnlde has another idea?!?

I don’t want to use https://github.com/yeoman/grunt-usemin because it’s unmaintained.

@reneviering
Copy link
Author

Thanks for your feedback.
Yes, defining it inside the package.json feels better than inside the Gruntfile. I think i'll give it a try.

Am i right that CSS-bundling is only relevant for dependencies outside bootstrap? Because bootstrap is not treated as a vendor, it's included in our client main less file? Once a library brings less it can be integrated by including it in the main client less file!?
If not, the css files are bundled with parcelify by defining its files inside the style-property inside the package.json !?

@mischah
Copy link
Member

mischah commented Apr 5, 2016

My pleasure 🐨

Am I right that CSS-bundling is only relevant for dependencies outside bootstrap?

Jep.

Because bootstrap is not treated as a vendor, it's included in our client main less file?

Jep. We kinda build or own bootstrap dist with the approach how we create templates by using the Bootstrap sources.

Once a library brings less it can be integrated by including it in the main client less file!?
Good question. We weren’t consistent when it comes to include vendor less files in the past. I guess we should discuss that elsewhere 😙

If not, the css files are bundled with parcelify by defining its files inside the style-property inside the package.json !?

Yeah. At least that’s like I understood the approach like described over here. Some maintainers already use the style property in their packages like this one for example: https://github.com/LukyVj/Colorify.js/blob/master/package.json#L17-L19

René Viering added 6 commits April 6, 2016 15:37
* develop:
  Release v2.1.2
  Fix travis error regarding Java version
  Update jQuery to the latest 1.x
  Run `.postinstall.js`explicitly with node
  Upgrade dev dependencies
  Add René Viering

# Conflicts:
#	package.json
@reneviering
Copy link
Author

As discussed yesterday with @mischah, i added a few commits handling css for frontend-dependencies.

Instead of parcelify, css files which belong to a frontend-dependency like »select2« are now added to the package.json property »frontendDependenciesCSS«.

These defined css-files are automatically added to a »libs.min.css« in both directories, server and dist.

import $ from 'jquery';

// this is necessary because bootstrap itself checks the existence of jQuery with window.jQuery.
window.jQuery = $;
Copy link
Member

Choose a reason for hiding this comment

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

Couldn’t we do a import jQuery from 'jquery'; instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes you could, but that wouldn't make it global.

@reneviering
Copy link
Author

I just removed the dependencyConfiguration module. Instead of this i use a simple function inside the Gruntfile to get the additional css files of frontend dependencies like »select2«.
Furthermore i renamed the attribute inside the package.json to »bundleCSS«.

As discussed with @mischah today, we can now merge this branch into develop. I would suggest implementing the other improvements like »optimizing bundle file size« in separate feature branches after the merge!?

@mischah
Copy link
Member

mischah commented Apr 18, 2016

Jep. Ready to merge 👌

I would prefer a rebase to get rid of a few commits to maintain a cleaner history (Especially for the changelog). 😘

@reneviering reneviering self-assigned this Apr 19, 2016
@mischah
Copy link
Member

mischah commented Apr 19, 2016

Rebased with a few squashs and merged in develop 🎉 🎈

@mischah mischah closed this Apr 19, 2016
@mischah mischah changed the title [WIP] Feature/es6 modules Feature/es6 modules May 11, 2016
@mischah mischah deleted the feature/es6-modules branch March 24, 2017 12:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants