Skip to content

feat(component): add less support #115

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

Closed
wants to merge 1 commit into from

Conversation

wesleycho
Copy link
Contributor

  • Add optional support for less via style option

{ name: 'verbose', type: Boolean, default: false, aliases: ['v'] },
{ name: 'blueprint', type: String, default: 'ng2', aliases: ['b'] },
{ name: 'name', type: String, default: '', aliases: ['n'] },
{ name: 'style', type: String, default: 'css', aliases: ['s'] }
Copy link
Member

Choose a reason for hiding this comment

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

we need a test for this

@cironunes
Copy link
Member

Thanks @wesleycho, I liked the approach.
We need tests for the generate command and I added a few comments, otherwise it's good to merge.

@wesleycho
Copy link
Contributor Author

Alright, I'll take a look today at writing the tests - might not get to it for a few days.

The approach is a bit hackish since it rewrites the file name after install, but I couldn't think of anything better.

@cironunes
Copy link
Member

Yes, we might have template files in the future, but I'm ok with this for now.

@wesleycho
Copy link
Contributor Author

I added tests, still need to implement that change to only compile the less if less is present, pending answer to question.

- Add optional support for less via `style` option
@filipesilva
Copy link
Contributor

There was been talk of a angular2-config.json or a similar file. This sort of configuration would belong there I'd say.

@filipesilva
Copy link
Contributor

Also, there seems to be some trouble with newly generates files #120 (comment)

@Brocco
Copy link
Contributor

Brocco commented Jan 25, 2016

As of now the idea of configuration has been moved into package.json adding the style option should be placed here: https://github.com/angular/angular-cli/blob/master/addon/ng2/blueprints/ng2/files/package.json#L5-L9

@intellix
Copy link
Contributor

just wondering how this is optional support (as I'd like to have SASS instead) :)
broccoli-less is added to package.json and the LESS compilation is added to angular2-app.js. For this to be optional, wouldn't it have to be added as an additional addon that was separate to this project?

@hansl
Copy link
Contributor

hansl commented Feb 12, 2016

This feels like something that we should support properly but need design beforehand.

We need to let the user choose, and I think ng install is the right place for this logic. Using something like ng install less for example, changing the build and the scaffolding, would be great. From that point the user would create components that use less and build less when he runs ng serve. I could also see an ng install sass coming after the first one and we generate an error with something like "you are already using a build library that targets the same kind of files; use both? Y/n".

I'm reworking the specs of ng install right now and this particular usage will be taken into account.

I think this PR should be closed as this will never be something we support natively. There should not be a force of technology that the user has to pick (outside of the cli itself, of course). I could even see a point where we don't force the user to use TypeScript if he doesn't want to, and generate JavaScript components. I had a ng use kind of command for this, but this could be built into ng install.

@hansl hansl closed this Feb 12, 2016
@wesleycho
Copy link
Contributor Author

I'm in agreement - even my refactor I have locally still is a bit incorrect.

This probably needs a whole spec and maybe a plugin implementation.

@wesleycho wesleycho deleted the feat/less-support branch February 12, 2016 22:57
@hansl
Copy link
Contributor

hansl commented Feb 12, 2016

Wes, you'll be in on the discussion for the ng install, and you'll definitely be welcome to do any work necessary to support less.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants