-
-
Notifications
You must be signed in to change notification settings - Fork 263
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
feat(platform): add initial Nx plugin support #308
Conversation
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for analog-app ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for analog-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
2192e3a
to
ef8f59f
Compare
@brandonroberts moved the plugin into the We are able to get an application up and running with Documentation on this is still missing for now. I wanted to ask you for your input on that. Where does it make the most sense to put it? As part of the Getting Started page or somewhere else? |
packages/platform/src/lib/nx-plugin/executors/build/executor.ts
Outdated
Show resolved
Hide resolved
packages/platform/src/lib/nx-plugin/generators/app/generator.ts
Outdated
Show resolved
Hide resolved
packages/platform/src/lib/nx-plugin/generators/app/generator.ts
Outdated
Show resolved
Hide resolved
packages/platform/src/lib/nx-plugin/generators/app/generator.ts
Outdated
Show resolved
Hide resolved
packages/platform/src/lib/nx-plugin/generators/app/generator.ts
Outdated
Show resolved
Hide resolved
packages/platform/src/lib/nx-plugin/generators/app/generator.ts
Outdated
Show resolved
Hide resolved
Is there a reason why the |
Had a discussion about this also. My thoughts: The platform package is the "glue" for cohesive support. If it's in the platform package, we can use one package for generating + upgrading Angular CLI and Nx workspaces. nx migrate @analogjs/platform Using |
It (the nx-plugin lib) doesn't necessarily have to stay in Check https://github.com/angular-threejs/angular-three where I have |
Ahh I see. I see things under Thoughts @goetzrobin on keeping it as separate "package" but shipping it inside of |
@brandonroberts it is mainly for ergonomics as well as prevent potential circular dependency in the future where graph TD;
plugin-->package-A;
package-A-->package-B;
package-B--contains-->plugin;
I personally don't feel too strongly about it. |
@brandonroberts I am open to either solution. I think the chance of the circular dependency is fairly low because the plugin has such a specific use case? Really comes down to what you prefer, you guys have much more experience publishing libraries. (I have 0 😄 ) |
@goetzrobin @nartc I'd say keep it as a separate package like you had before, and just distribute it as part of the You should be able to revert the last commit or hard reset the branch back to the previous commit, whichever is easier for you. |
@goetzrobin if you have any question at all, please don't hesitate to reach out |
…rmalized options/ versions
216e618
to
187cb1d
Compare
@brandonroberts @nartc @MDyrcz5 I reset the branch to before I moved it and implemented the suggestions. I'll need some help figuring out how to package them together and what needs to happen for it to work (merge package.json or anything else I am missing.) Feel free to add changes/documentation as you see fit! |
~~Nice work! We'll have to see how this works on a fresh Nx workspace. My thought is that it works here because we have all the Vite dependencies installed but a fresh workspace won't. We may need to run the That can be done in a follow-up though.~~ I'll take another look and we can get this merged in to move forward. 🎉 |
@brandonroberts it should install the necessary dependencies if they don't already exist when running the generator. The e2e test generates a whole new workspace in the tmp folder. The biggest question for me right now is how we package them together but I am sure @nartc already has an answer for that 🤓 |
@goetzrobin you're right. I missed it scrolling through the changes. I can take a look at the build part. Essentially, we'll have a When we run the build, it will build the platform package, then run the build for the plugin and put it inside the folder of the built platform package |
@goetzrobin Several steps:
|
Thanks all! @allcontributors add @MDyrcz5 for code |
I've put up a pull request to add @MDyrcz5! 🎉 I've put up a pull request to add @nartc! 🎉 |
Nx generator support has been released in |
…ress
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Which package are you modifying?
What is the current behavior?
No Nx generator exists
Issue Number: #293
What is the new behavior?
Adds Nx generator to the project. In an Nx workspace with
@analogjs/platform
installed we can donx g @analogjs/platform:app hello-world
to create a new analog application.Does this PR introduce a breaking change?
Other information