-
Notifications
You must be signed in to change notification settings - Fork 156
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
Migrate UI Primitives to be secondary entry points of a single package instead of individual dependencies #476
Comments
Thanks, yes this will be great improvement. Obviously there will be one task to create the new library, but what way should we undertake moving each existing library - do we have one person do it all at once? Or do we split it up on a per feature basis similar to how we have dealt with the signals? Lots of smaller PRs might allow things to be migrated in parallel and easier to resolve conflicts as we are still working through signal migrations? Downside is lots of PRs to review. Open to thoughts! One other thing that we could tie in here is linting. If we decide ESLint would be the tool of choice, we could create the new library with ESLint, have that run during CI, and as we move each feature ensure that linting passes. Might allow us to "kill two birds with one stone". |
I think we should migrate 1 package/primitive and it can be the example on how we wish to do the others if we wish to do it on a per feature basis. The plus is no one person is burdened with the whole thing. More PR's but they will at least be smaller PR's so it will be easier to review and catch any issues. In general we should probably add some content about the package change on site and just mention how to migrate as well. Might be a stretch but we could add something in cli to help with migration. Might be easier said than done but might be worth looking into. But yea open to ideas on how we want to tackle this |
I agree, that would be my vote too.
Yeah, when I went through this process with a library before there were two options we came up with:
export * from '@spartan-ng/brain/dialog'; This way you don't need to pollute your source with backwards compat packages. There may be others, but those are the two we considered. In the end we chose a migration and that worked very well for us, so that would probably be my vote. Creating stub packages is just kicking the can down the road In my opinion. But Im open to hearing any ideas other may have! |
@ashley-hunter I saw your comment on the calendar PR about taking a crack at this so I assigned the issue just to keep track. Does that sound good? |
Perfect 👌 thank you! |
Which scope/s are relevant/related to the feature request?
Don't know / other
Information
Currently each UI primitive must be installed as it's own dependency in a project. The core team discussed how we can best manage versioning and in considering both DX and maintainability from our end, we have decided it would be easier to have a single package root that would need to be installed and version managed.
We will create secondary entry points for each UI primitive allowing for users to still incrementally adopt a library if they wish or at least be certain only what is used by your project is included in the production build. We believe this will provide the best DX in the end, by only needing to be concerned with a single version and only 1 package.json dep entry. This will ultimately help maintainability and our support for the project as well as have the added the benefit of streamlining the project's release process.
For reference this would work much the same way other official angular packages work like for example angular cdk package and only wanting to import the dialog -> @angular/cdk/dialog.
Tentatively for this project this might look like @spartan-ng/brain/dialog for example instead of needing to have @spartan-ng/brain/dialog and @spartan-ng/helm/dialog
Describe any alternatives/workarounds you're currently using
No response
I would be willing to submit a PR to fix this issue
The text was updated successfully, but these errors were encountered: