Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
lurock merged 1 commit into SoCreate:master from sulco:feature/cli-schematics
Aug 21, 2018
Merged

Conversation

@sulco
Copy link
Contributor

@sulco sulco commented Jul 29, 2018
edited
Loading

Adds ng-add (削除) and sandbox (削除ここまで) (sandbox part extracted into separate PR) schematics (addresses issue #73)

ng-add will be executed when you run ng add angular-playground (once this is published to npm) and will:

  • add angular-playground to package.json,
  • run npm install,
  • create main.playground.ts and angular-playground.ts
  • and add playground npm script to package.json

In order to try this functionality locally, from the angular-playground/packages/angular-playground directory run:

  1. npm run build
  2. npm link

Then create a sample Angular project wherever (ng new playground-dummy) and in that directory run

  • npm link angular-playground
  • ng add angular-playground

important notice: since ng add angular-playground runs npm install, this will, in our testing scenario, overwrite the linked playground package with the npm one. At this point you should be able run npm 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 SandboxOfConfig configuration also generated.

Copy link
Contributor

jschwarty commented Jul 30, 2018
edited
Loading

I was just getting around to making a similar PR. Nice @sulco! Some thoughts:

  • The playground project can have a main.ts file instead of a unique named main.playground.ts one.
  • The projects.playground.architect.serve.options can 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).

Copy link
Contributor Author

sulco commented Aug 2, 2018

Thanks for the feedback @jschwarty! Makes sense - I'll split the PR then and implement the suggestions for ng add (likely tomorrow or Monday)

Copy link
Contributor

greg-arroyo commented Aug 7, 2018
edited
Loading

@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?

Copy link
Contributor Author

sulco commented Aug 9, 2018

Hey @arroyocode, yes, I will have some time today to update the PR.

@sulco sulco force-pushed the feature/cli-schematics branch from 27a859d to ec13d9c Compare August 9, 2018 22:42
Copy link
Contributor Author

sulco commented Aug 9, 2018
edited
Loading

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 :)

Copy link
Contributor

Again, thank you @sulco for your contribution to playground. We'll begin reviewing internally shortly along with your other PR adding sandbox schematics.

Copy link
Contributor

jschwarty commented Aug 10, 2018
edited
Loading

@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?

😎

Copy link
Contributor Author

sulco commented Aug 10, 2018
edited
Loading

@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?

Copy link
Contributor

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

Copy link
Contributor

@timstoddard timstoddard left a 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`,
Copy link
Contributor

@timstoddard timstoddard Aug 10, 2018

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`,
Copy link
Contributor

@timstoddard timstoddard Aug 10, 2018

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,
Copy link
Contributor

@timstoddard timstoddard Aug 10, 2018

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'

@sulco sulco force-pushed the feature/cli-schematics branch from ec13d9c to adc7dec Compare August 11, 2018 11:12
Copy link
Contributor Author

sulco commented Aug 11, 2018

@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)

@lurock lurock merged commit 9d9295e into SoCreate:master Aug 21, 2018
@sulco sulco deleted the feature/cli-schematics branch August 21, 2018 22:05
danielkresta pushed a commit to danielkresta/angular-playground that referenced this pull request Oct 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lurock lurock lurock approved these changes

+1 more reviewer

@timstoddard timstoddard timstoddard approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /