-
Notifications
You must be signed in to change notification settings - Fork 61
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:
Also, maybe the schematic support for |
|
Thanks for the feedback @jschwarty! Makes sense - I'll split the PR then and implement the suggestions for |
|
@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 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 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 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 |
|
You would replace the contents of |
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.
|
|
||
| const project: Partial<WorkspaceProject> = { | ||
| root: `${projectRoot}`, | ||
| sourceRoot: `${projectRoot}src`, |
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.
| fileReplacements: [ | ||
| { | ||
| replace: `${projectRoot}src/environments/environment.ts`, | ||
| with: `${projectRoot}src/environments/environment.prod.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.
This file should be suffixed with .ts instead of .t
| */ | ||
| export function addPackageToPackageJson( | ||
| host: Tree, | ||
| type: string, |
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 @jschwarty 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 |
add cli schematics SoCreate#73
Adds
ng-addand(sandbox part extracted into separate PR) schematics (addresses issue #73)sandboxng-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.