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

Add possibility to exclude certain parts from the build #13

Merged
merged 4 commits into from
Dec 6, 2017

Conversation

anehx
Copy link
Member

@anehx anehx commented Nov 30, 2017

This PR adds some options and fixes #12:

'ember-uikit': {
  importUIkitCSS: true, // excludes css from build if false
  importUIkitJS: true, // excludes js from build if false
  importUIkitAssets: true, // excludes assets (not icons) from build if false
  importUIkitIcons: true, // excludes icons from build if false

  useIcons: true, // excludes all icon related stuff if false

  whitelist: [], // components which will be included
  blacklist: [] // components which will be excluded
}

TODO

  • Document options
  • Exclude uk-icon if useIcons: false

@anehx anehx requested a review from czosel November 30, 2017 13:19
@anehx anehx changed the title Add possibility to exclude certain parts from the build [WIP] Add possibility to exclude certain parts from the build Nov 30, 2017
@anehx anehx changed the title [WIP] Add possibility to exclude certain parts from the build Add possibility to exclude certain parts from the build Dec 5, 2017
@anehx anehx mentioned this pull request Dec 5, 2017
@jayvarner
Copy link
Contributor

The treeshake pattern is very elegant. Documentation seems clear. Nice work!

@anehx anehx mentioned this pull request Dec 6, 2017
<td>
A list of included components. Only components in this list will be
included in your build. You should never use <code>whitelist</code>
and <code>blacklist</code>!
Copy link
Contributor

Choose a reason for hiding this comment

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

If you say that the two lists should not be used at the same time, i'd expect an error to be thrown in that case. I think right now the blacklist takes precedence.

Copy link
Member Author

Choose a reason for hiding this comment

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

That should definitely throw an error!

Copy link
Member Author

Choose a reason for hiding this comment

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

Or at least a warning

Copy link
Contributor

@czosel czosel left a comment

Choose a reason for hiding this comment

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

Apart from the nitpick, LGTM!

@anehx anehx merged commit 58e3c72 into adfinis:master Dec 6, 2017
@anehx anehx deleted the feature/options branch January 3, 2018 09:15
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.

Include icons optional
3 participants