- 
  Notifications
 
You must be signed in to change notification settings  - Fork 62
 
add cli schematics #73 #126
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
Conversation
I was just getting around to making a similar PR. Nice @sulco! Some thoughts:
- The playground project can have a 
main.tsfile instead of a unique namedmain.playground.tsone. - The 
projects.playground.architect.serve.optionscan get a "port" property and that set to 4201 by default. (although this might not really come into play until there is a "builder" to run the playground cli script via the architect) 
Also, maybe the schematic support for ng add and the generate schematics can be two separate PRs as the generate ones will be more opinionated and would probably garner more discussion before implementation (like should the sandbox schematic create a .sandbox.ts file next to the component file or in a sandbox directory).
Thanks for the feedback @jschwarty! Makes sense - I'll split the PR then and implement the suggestions for ng add (likely tomorrow or Monday)
@jschwarty thank you for the suggestions on this! @sulco, just curious if you're still planning to implement suggestions or should we be reviewing as is based on the time frame indicated by your comment?
Hey @arroyocode, yes, I will have some time today to update the PR.
27a859d to
 ec13d9c  
 Compare
 
 Following the suggestion I've extracted the sandbox creation part into a separate PR, this one will deal with ng-add only. I've also updated the workspace configuration to include the port: 4201 entry.
I've left main.playground.ts to maintain a consistency with the current installation guide (http://www.angularplayground.it/docs/getting-started/angular-cli).
This one should be ready for review :)
Again, thank you @sulco for your contribution to playground. We'll begin reviewing internally shortly along with your other PR adding sandbox schematics.
@sulco @arroyocode I think the ng add should make use of the default main.ts file that an Angular CLI application creates instead of making a different main file (since there is no need for two mains when the Playground is set up as a separate application project in the workspace). The getting started docs would get updated to reflect using the ng add command with the Angular CLI and all of the stuff about setting it up in an Angular CLI workspace (including the instructions about main.playground.ts) would be obsolete.
The schematics set to generate sandbox files and such is something that end users can decide to use or not. They can always make their own or extend the Playground one if so desired.
The schematic for ng add is not something they can create on their own, it has to live with the Playground package. It would be ideal if that aligned itself with the Angular CLI patterns. If it doesn't, the only way end users can make it so would be a manual install and setup...they would not be able to use ng add.
Make sense?
😎
@jschwarty @arroyocode the strategy makes sense, I just think I need to wrap my head around not having the dedicated entry file (That was a one place where I could load just stuff for "playgrounding" there + some mocks all at once).
Aaanyways. Could you guide me to as to how exactly this should be set up? When I replace main.playground.ts with main.ts it just loads my main app instead of the Playground. How does PlaygroundModule get bootstrapped in the new scenario?
You would replace the contents of main.ts with the contents of main.playground.ts. I just did an episode on AngularAir where I showed how to set it up in a CLI 6+ project. https://youtu.be/lq_EngmFrbU 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @sulco, I tested the PR on my local machine and the "ng add" functionality works as expected, nice job on that! There's a few small things that I've noted that need to be fixed before we can merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and all other usages of ${projectRoot} should have a slash after them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be suffixed with .ts instead of .t
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make the type of type more specific: type: 'dependencies' | 'devDependencies'
ec13d9c to
 adc7dec  
 Compare
 
 @timstoddard 
Good points! I've made the requested changes.
@jschwarty 
Thanks for the video, haven't got to see that before yet. I'm still not sure I'm grasping the idea though: so now we will have a separate project for some of the playground files? It seems like a big overhead (a lot of unnecessary(?) files + a bit awkward paths back to the "primary" project)
Plus conceptually it sandboxes always seemed similar to unit tests, so a part of a primary project, with files being close by the components. The "center of gravity", so to speak, is in the main project, so why yanking out playground's entry file of it?
Don't get me wrong, best intentions on my part, but I can't understand the merit of this solution in comparison to the current, simpler one. I probably lack some context or enough use cases to see the allure of it.
So.
If the current setup does not work for the planned development of Playground, could you help me, eg. by specifying the directories and files that should be added with the ng add command? (just the structure, I thin I'll be able to figure out the code).
Or should that be the whole output of ng generate application command? (seems excessive, but again, just an opinion)
add cli schematics SoCreate#73
Uh oh!
There was an error while loading. Please reload this page.
Adds
ng-add(削除) and(sandbox part extracted into separate PR) schematics (addresses issue #73)sandbox(削除ここまで)ng-addwill be executed when you runng add angular-playground(once this is published to npm) and will:package.json,npm install,main.playground.tsandangular-playground.tsplaygroundnpm script topackage.jsonIn order to try this functionality locally, from the
angular-playground/packages/angular-playgrounddirectory run:npm run buildnpm linkThen create a sample Angular project wherever (
ng new playground-dummy) and in that directory runnpm link angular-playgroundng add angular-playgroundimportant notice: since
ng add angular-playgroundrunsnpm install, this will, in our testing scenario, overwrite the linked playground package with the npm one. At this point you should be able runnpm run playground, but the schematics are gone.--
Please do bug me if you have any questions or doubts :) I intended this PR to be a MVP for this functionality that we can build upon once it's live and kicking.
e.g. when generating a sandbox one could specify a list of scenarios (so that you don't have to write "storyOf" by hand) or to have
SandboxOfConfigconfiguration also generated.