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 support for new targets feature in ember-cli. #34

Merged
merged 1 commit into from Apr 22, 2017

Conversation

rwjblue
Copy link
Contributor

@rwjblue rwjblue commented Apr 19, 2017

See:

On older ember-cli versions project.targets is undefined, which would cause the default autoprefixer results (same as prior to this change).

On newer ember-cli versions the projects own targets are used and manual configuration in ember-cli-build.js is not needed.

Closes #32.

See:

* https://github.com/ember-cli/rfcs/blob/master/complete/0095-standardise-targets.md
* https://emberjs.com/blog/2017/03/19/ember-2-12-released.html#toc_targets

On older ember-cli versions `project.targets` is undefined, which would cause
the default autoprefixer results (same as prior to this change).

On newer ember-cli versions the projects own targets are used and manual configuration
in `ember-cli-build.js` is not needed.
@rwjblue
Copy link
Contributor Author

rwjblue commented Apr 19, 2017

/cc @cibernox @kimroen

Copy link

@cibernox cibernox left a comment

LGTM.

@@ -6,20 +6,25 @@ var defaults = require('lodash/defaults');

module.exports = {
name: 'ember-cli-autoprefixer',
included: function(app, parentAddon) {

included: function(app) {

Choose a reason for hiding this comment

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

If we're node 4 we can use shorthand syntax.

Copy link
Owner

@kimroen kimroen Apr 22, 2017

Choose a reason for hiding this comment

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

You're right, but that sounds like something that should be handled separately.

this.options = defaults(this.app.options.autoprefixer || {}, {
browsers: this.project.targets && this.project.targets.browsers,

Choose a reason for hiding this comment

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

It's super convenient that both use the same syntax.

@cibernox
Copy link

cibernox commented Apr 19, 2017

How complex would be to add a smoke test that uses two fixtures and mimics the addon being invoked with different targets to see that the output is different?

@rwjblue
Copy link
Contributor Author

rwjblue commented Apr 21, 2017

@kimroen - Friendly ping 😸

@kimroen kimroen merged commit d339c9b into kimroen:master Apr 22, 2017
1 check passed
kimroen added a commit that referenced this issue Apr 22, 2017
@kimroen
Copy link
Owner

kimroen commented Apr 22, 2017

Thank you so much - good work everyone!

Released as v0.7.0: https://github.com/kimroen/ember-cli-autoprefixer/releases/tag/v0.7.0

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.

3 participants