-
Notifications
You must be signed in to change notification settings - Fork 12k
Proposal: Auto add (providers, directives, pipes, styles, styleUrls) when adding 3rd party libs, etc. #96
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
Comments
the proposal sounds reasonable to me (with some tweaks). I think that we should prefer local import over global import, so I'd reorder the options. Would you like to create a test package(s) in npm that we can experiment with? |
adjective:
_programmers context_:
Might be a fun command: |
i think perhaps a simple solution for this would be something along the lines of //myPluginRoot.ts
import {MyPluginServiceA, MyPluginServiceB} from './services';
import {MyDirectiveA, MyDirectiveB} from './directives';
import {MyPipeA, MyPipeB} from './pipes';
//export the named stuff for granular usage
export * from './services'
export * from './directives'
export * from './pipes'
//use es6 default syntax to export the "plugin"
export default {
providers: [MyPluginServiceA, MyPluginServiceB],
directives: [MyDirectiveA, MyDirectiveB],
pipes: [MyPipeA, MyPipeB]
} users and tooling are then easily able to do : //naming is irrelevant here
import myPlugin from 'myPlugin'
//now its consistent:
myPlugin.directives
myPlugin.services
myPlugin.pipes and this is easily done with templating, as the actual name of the export is "default" - no weird lookups necessary and no NAMING_CONVENTIONS to force / invent. see http://plnkr.co/edit/Yi1e3ifiX0gSqd8Qs2h4?p=preview - i added a little plugin registration helper too, which @IgorMinar will probably yell at me about. |
That's the best solution I've heard yet @robwormald. Very clean and simple. I could pull your plunk (the gist of it anyway) into a repo tomorrow or |
@robwormald I like it very much |
+1 on this solution @robwormald :) |
@robwormald — Brilliant, love your proposal! 👍 |
Go for it! (maybe not the helper thing...)
|
Will not this push to use MVC like folder separation? +1 to @robwormald that we should not push/invent name conventions PS: I am never using generators, and not allow to, guys should understand what and why they are doing :) |
this is for third party packages (like yours) which will presumably ship via NPM, and thus the structure of the package is mostly irrelevant to the user. The structure internal to the module / plugin is entirely up to you, that was just a quick and dirty demo. |
@valorkin — I think using a folder structure with The structure for an actual application (not a library) would be component driven though, if that makes sense. |
just said my IMHO |
@valorkin — I think the Angular repository (in its current form) is hard to compare to a third party library. However, if we peek inside one of the modules, then we can see a similar structure e.g.: https://github.com/angular/angular/tree/master/modules/angular2/src/common I think such a structure is probably closer to what most (smaller) libraries will contain. If you have a larger library that contains modules then it definitely makes sense to create a module folder structure. Ultimately the library author should decide. Although one could argue that it would be better to split into different repositories than in different modules. Would be great if there is an Angular 2 ecosystem with a lot of very small libraries (as in node). What are your thoughts on splitting into different repositories? Thanks! |
Taking in account user option to to require and bundle only require part of libs modules are optional |
@IgorMinar Here is a test lib to experiment with that uses @robwormald's excellent standards proposal above:
Expectations of cli
Sample component to test if it worked
Given something like the above, you should be able to see this in the console:
In the browser, this should be seen: The The Conclusions
btw, @robwormald's registerPlugins (in |
@NathanWalker you just made best ng2 starter package for angular 2? You know it right? :) |
@NathanWalker — Great effort! Thanks! Once we have settled on a best practice I will also update generator-angular2-library accordingly. |
fyi, testing lib https://www.npmjs.com/package/ng2-cli-test-lib, updated to use latest alpha.53. @IgorMinar @robwormald if either of you have any opinion on lib name prefix (repo prefix), please share. Most of the discussion has settled on |
no real comment on the prefix. thinking about the installation process - i'd personally rather use npm. i think it would be reasonable to have package authors add a field to their package.json (something like |
I was thinking the same. Using npm manually is likely universally preferred. |
you should also add import ng2lib, {MyDirective} from 'ng2lib';
@Component({
selector: 'app',
providers: [ ...ng2lib.providers],
directives: [ ...ng2lib.directives],
styles: [ ...ng2lib.styles],
template: `<my-directive></my-directive>`
})
export class App {} parsing |
+1, fully integrated :) |
What if a library author would configure Possible benefits:
Possible issues:
|
What happens when the end user doesn't want the lib to auto install inside its application? |
@ocombe In that case, user could simply exit, Ctrl + c, since the install would have completed, they could just exit the @jvandemo great additional thoughts. Using cli manually could still work as suggested/described throughout this discussion and as an option on top of everything, libs could also provide this Possible issues:
|
Based on @gdi2290 suggestion to handle This works out like the following:
This attempts to handle
This may or may not be the best way forward, but wanted to give you something to experiment with here to handle inline styles as well as styleUrls. Test if styles were added properlyIf all the styles load properly (the inline as well as the style urls), then the directive should render in Additionally if the styleUrls considerationsFor this to work, a couple things will probably need to happen:
Lastly, note that the |
No, it should entirely be optional. |
Agreed, maybe any npm script that is not triggered automatically could do e.g.:
Then it would be optional and the benefits of the script remain. It would also solve the dependency issue where the consumer would have to have |
Great work Nathan! What is your reasoning behind using a static method for styles? |
@jvandemo thanks, it's really just 1 way to do it, no particular reason, On Mon, Dec 14, 2015 at 11:34 PM Jurgen Van de Moere <
|
@NathanWalker Interesting, because using a function could be convenient to pass in options e.g.: styles: TestStyles.styles({
flexbox: true
}) |
@jvandemo +1 would provide for some very interesting app configurations across various devices ;) |
https://www.npmjs.com/package/ng2-cli-test-lib updated to |
Turns out a really cool guy @superchris tried out the lib today on his own brave accord and in the process helped reveal a couple things, of which I just fixed in His project shows an example implementation of the One thing of interest is that the However Given this, when integrating the library in, I had to do this: Lastly, until the |
we should be using spread operator whenever possible since it also clearly shows which ones are collections and which ones aren't (as if all caps wasn't enough) import {FORM_DIRECTIVES} from 'angular2/common';
import {APP_PROVIDERS} from '../app-providers';
import {MyDirective} from './my-directive';
import {APP_STYLES, APP_THEME_STYLES} from '../app-styles';
@Component({
selector: 'home',
providers: [
...APP_PROVIDERS
],
directives: [
...FORM_DIRECTIVES,
MyDirective
],
styles: [
...APP_THEME_STYLES,
`
.main {
background: gray;
}
`
],
template: `
<my-directive></my-directive>
`
})
export class App {} for |
@gdi2290 +1 nice :) |
I got something working. See #135 |
Is it possible to add 3rd party library without adding mappings to system config? |
Please check out my last commit. Any suggestions/tests/fixes/comments are very welcome. If someone interested to check it out, here is usage example
then create new project using
I tested out and the project builds properly, with injecting with custom or automatic injection or uninstalling the lib afterwards (cleans properly). If this will hopefully be accepted by maintainers we can define how the library must be created to be compatible with |
The more I think about this idea, the less I like it, at least in regards to automatic code generation and injection and the like. I think it's an overreach for what the CLI should do. |
I can certainly understand that. To me, cli tools in general are merely a toolbox containing commands that The goal here was to simply offer an additional helpful aid to those that Personally, this is the type of tool in the toolbox that I would use for I think most advanced users (pros) always have a sour taste for tools that In the end, it's just about helpful options to me. Ya like honey on your On Tue, Jan 12, 2016 at 1:44 AM Rob Wormald [email protected]
|
@robwormald you can simply answer "no" to the first question after installing a lib with that command/task. |
This has been merged. Documentation is here: #173 |
I am using it with angular4 and getting error with ng serve that 'ERROR in ImageLazyLoadModule is not an NgModule'? |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Discussion on AngularAir today regarding 3rd party libraries (https://plus.google.com/events/c7m79d5o04nom5eednk5ecqtebk) brought up an interesting thought.
See also: angular/angular#5503
If 3rd party libs all adhered to a certain naming convention and style when exporting necessary services, directives, pipes, etc., it would be incredibly helpful if this CLI could auto-annotate the src code files with the necessary
directives
,providers
,pipes
, etc.Consider the following scenario:
ng2-image-lazy-load
(https://github.com/NathanWalker/ng2-image-lazy-load)IMAGELAZYLOAD_PROVIDERS
, the CLI might provide an additional helpful aid (command can be whatever, just usingadd-lib
for example):It would proceed to run
npm install ng2-image-lazy-load
, then prompt with a couple questions:If "y":
It would add the import statements and append the aptly named
[lib_prefix]_PROVIDERS
in the user's app bootstrap.If the lib contained no constants with
PROVIDERS
in the name, it would modify nothing.If "n":
It would add the import statements and append the libraries aptly named
PROVIDERS
in theviewProviders
of the component.Similar scenarios would apply for
DIRECTIVES
,PIPES
, etc.If this were possible, every 3rd party lib could suggest using the CLI as the first option to install, followed by manual npm install.
/cc @jvandemo, @gdi2290, @valorkin, @ocombe, @jeffwhelpley, @PascalPrecht, @mgechev, @ludohenin, @robwormald
The text was updated successfully, but these errors were encountered: