Skip to content
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

Merged
merged 3 commits into from
Mar 30, 2023
Merged

Conversation

goetzrobin
Copy link
Member

@goetzrobin goetzrobin commented Mar 27, 2023

…ress

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Which package are you modifying?

  • vite-angular-plugin
  • astro-angular
  • create-analog
  • router
  • platform
  • content

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 do nx g @analogjs/platform:app hello-world to create a new analog application.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@netlify
Copy link

netlify bot commented Mar 27, 2023

Deploy Preview for analog-blog ready!

Name Link
🔨 Latest commit 187cb1d
🔍 Latest deploy log https://app.netlify.com/sites/analog-blog/deploys/64249fddb5929000079d8354
😎 Deploy Preview https://deploy-preview-308--analog-blog.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 27, 2023

Deploy Preview for analog-app ready!

Name Link
🔨 Latest commit 187cb1d
🔍 Latest deploy log https://app.netlify.com/sites/analog-app/deploys/64249fdda696a3000851d5ac
😎 Deploy Preview https://deploy-preview-308--analog-app.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 27, 2023

Deploy Preview for analog-docs ready!

Name Link
🔨 Latest commit 187cb1d
🔍 Latest deploy log https://app.netlify.com/sites/analog-docs/deploys/64249fdd29f05e0008fecbf5
😎 Deploy Preview https://deploy-preview-308--analog-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@brandonroberts brandonroberts changed the title feat(nx-plugin): add initial setup of nx-plugin. still a work in prog… feat(platform): add Nx plugin support Mar 29, 2023
@goetzrobin
Copy link
Member Author

@brandonroberts moved the plugin into the platform package!

We are able to get an application up and running with nx g @analogjs/platform:app hello-world and nx run hello-world:serve. We can also successfully build it with nx run hello-world:build.

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?
Looking forward to reading your thoughts! 🚀 👍

@nartc
Copy link
Member

nartc commented Mar 29, 2023

moved the plugin into the platform package!

Is there a reason why the plugin needs to be part of platform package?

@brandonroberts
Copy link
Member

moved the plugin into the platform package!

Is there a reason why the plugin needs to be part of platform package?

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
ng update @analogjs/platform

Using @analogjs/nx makes it seem like it doesn't work outside of Nx, but it does and should. It can still be listed as an Nx plugin, as it has all the necessary configuration supported.

@nartc
Copy link
Member

nartc commented Mar 29, 2023

It (the nx-plugin lib) doesn't necessarily have to stay in platform to be released together with platform.

Check https://github.com/angular-threejs/angular-three where I have plugin and angular-three as two separate Nx projects. Running build in angular-three also builds plugin and the outputPath controls where plugin gets built to.

@brandonroberts
Copy link
Member

Ahh I see. I see things under packages as things that get shipped to npm. What other advantages are there of it being a separate package/lib?

Thoughts @goetzrobin on keeping it as separate "package" but shipping it inside of @analogjs/platform?

@nartc
Copy link
Member

nartc commented Mar 29, 2023

@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;
Loading

I personally don't feel too strongly about it.

@goetzrobin
Copy link
Member Author

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

@brandonroberts
Copy link
Member

@goetzrobin @nartc I'd say keep it as a separate package like you had before, and just distribute it as part of the platform package. It could simplify the builds of the two things, and let us keep the standard structure of an Nx plugin.

You should be able to revert the last commit or hard reset the branch back to the previous commit, whichever is easier for you.

@nartc
Copy link
Member

nartc commented Mar 29, 2023

@goetzrobin if you have any question at all, please don't hesitate to reach out

@goetzrobin goetzrobin marked this pull request as ready for review March 29, 2023 20:30
@goetzrobin
Copy link
Member Author

goetzrobin commented Mar 29, 2023

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

@brandonroberts
Copy link
Member

brandonroberts commented Mar 29, 2023

~~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 @nrwl/vite:init generator before generating a new app as it takes care of installing all the Vite deps.

That can be done in a follow-up though.~~

I'll take another look and we can get this merged in to move forward. 🎉

@goetzrobin
Copy link
Member Author

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

@brandonroberts
Copy link
Member

brandonroberts commented Mar 29, 2023

@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 build-plugin target and a build target.

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

@nartc
Copy link
Member

nartc commented Mar 29, 2023

@goetzrobin Several steps:

@brandonroberts brandonroberts changed the title feat(platform): add Nx plugin support feat(platform): add initial Nx plugin support Mar 30, 2023
@brandonroberts brandonroberts merged commit 3a84ced into main Mar 30, 2023
@brandonroberts brandonroberts deleted the create-nx-plugin branch March 30, 2023 12:05
@brandonroberts
Copy link
Member

Thanks all!

@allcontributors add @MDyrcz5 for code
@allcontributors add @nartc for review

@allcontributors
Copy link
Contributor

@brandonroberts

I've put up a pull request to add @MDyrcz5! 🎉

I've put up a pull request to add @nartc! 🎉

@brandonroberts
Copy link
Member

Nx generator support has been released in @analogjs/platform@0.1.0-beta.20!

Villanuevand pushed a commit to Villanuevand/analog that referenced this pull request Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants