-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
[V15] Updated dotnet template for Umbraco Packages with Bellisima #17108
base: v15/dev
Are you sure you want to change the base?
[V15] Updated dotnet template for Umbraco Packages with Bellisima #17108
Conversation
Hi there @warrenbuckley, thank you for this contribution! 👍 While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:
Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution. If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request. Thanks, from your friendly Umbraco GitHub bot 🤖 🙂 |
This is because the RCL based one will be the only one going forward
If you were extending/customising the backoffice of a project in v13 and earlier, you could just add your code into a folder in App_Plugins - there was no need for front-end build steps as you deployed the source files. This package template really was a (very simple) start to people building actual packages: it wasn't needed by people looking to just customise a specific Umbraco backoffice. In v14+ you definitely do not want to have your source files for your own extensions in App_Plugins, and so I think that the template being proposed here is arguably a template for extending the backoffice, not for building a package. So in light of that would renaming the template from
It would also differentiate this template from community templates that are designed for developing packages - that come with test sites, and a head-start on publishing to nuget etc. Whereas this core template is showing good practise for extending the backoffice. That extension might turn into a package to be distributed, but equally it might just be for a specific Umbraco project. |
@LottePitcher the rename of the dotnet new template makes total sense to me. |
@warrenbuckley I think the tooling setup (Vite etc) is still a good idea as it gets you started a good way, and it's the approach that backend devs with less front-end experience will need more help with. I don't think anyone would recommend using VanillaJS unless you were just building something incredibly simple. |
As this will only scaffold an extension and not other bits for a package such as Nuget, Github Actions & other things needed to be done to ship out a package
… can differ between Windows, Linux/OSX
Will allow us to ignore the folder if or when doing a dotnet pack with a rule in CSProj
…sion for completions in VSCode
@umbraco/packages-team take a look at this PR please gang |
I would also appreciate any opinions from the front end gang @leekelleher @nielslyngsoe @madsrasmussen @iOvergaard as well to ensure this is a-okay or needs stuff doing. |
{ | ||
"name": "UmbracoExtension EntryPoint", | ||
"alias": "PROJECT_NAME_KEBABCASE_FOR_NPM.entrypoint", | ||
"type": "backofficeEntryPoint", |
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.
Looks like both would work, but the example you have in your entry point file looks more like the bundle
type than the backofficeEntryPoint
type. Maybe someone on the Backoffice team has thoughts on this.
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.
AFAIK Bundles is for registering manifests only, where an entrypoint you may do other logic inside it.
Such as to do the AUTH stuff with the C# OpenAPI spec or perhaps unregister/exclude extensions etc...
But lets wait for someone from the frontend core crew to chime in.
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.
@nielslyngsoe @iOvergaard any thoughts ?
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.
Hi @warrenbuckley
I would say use the EntryPoint for what the entry point is needed for, but do not use it to registere more manifests. For that its better to use the Bundle.
Main reason begin a bundle knows the extensions it registres. Meaning if later and that could be milliseconds later, it knows how to unregisters it self. Useful if someone decides to unregister it.
So my opinion would be, in order to be the most correct as you can, then make both, one bundle and one entry point so people can see how the can execute code up front and use the bundle to lead the way for more extensions. :-)
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.
You have got the facts correct, @warrenbuckley. I would recommend sticking with the backofficeEntryPoint
.
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.
Seems overkill don't you think @nielslyngsoe & @iOvergaard to have the following and is more concepts to teach/get across unless this really is THE WAY
umbraco-package.json -> backofficeEntryPoint -> bundle
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.
Just to confirm you got everything right in your above comment. :-)
But I would still suggest this style:
umbraco-package.json -> bundle -> backofficeEntryPoint
Meaning anything additionally begin added would go to the bundle:
umbraco-package.json -> bundle -> dashboard
In this way avoiding to use the entry-point to do registration of other extension-manifest. Meaning the entry-point is purely needed for the act of running some JS code. Then if people then dont need to run code, they can ignore that specific type.
I think that is the most correct style.
This is based on an interest in registering extensions via JS. If you dont mind doing it in JSON. Then you can skip the bundle, to do additional registrations as of the umbraco-package.json.
But my point begin, I'm not a fan of show casing people that they should registere extensions by calling extensionRegistry.registerMany
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.
Would be good to set the BASE_DIR option somewhere here (can't remember where). It ensures that assets imported through Vite's public
folder have the correct path applied.
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.
The only thing used for the public folder atm is for the umbraco-package.json
If you have an example @iOvergaard I will gladly add it in 😄
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.
If you add this to the config:
base: '/App_Plugins/MyLib'
Full example:
export default defineConfig({
build: {
lib: {
entry: ['src/main.ts'],
formats: ['es'],
},
sourcemap: true,
outDir: '../wwwroot/App_Plugins/MyLib',
emptyOutDir: true,
rollupOptions: {
external: [/@umbraco-cms\/backoffice\//],
},
},
base: '/App_Plugins/MyLib',
});
Then stuff like this works:
<img src="/logo.jpg" />
and this
import imageUrl from '/logo.jpg';
<img src=${imageUrl} />
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.
You could add a build step somewhere here to ensure frontend assets are built on startup. There are various ways of checking if they need to be rebuilt.
<PropertyGroup>
<ClientAssetsDirectory Condition="'$(ClientAssetsDirectory)' == ''">client/</ClientAssetsDirectory>
<ClientAssetsRestoreInputs Condition="'$(ClientAssetsRestoreInputs)' == ''">$(ClientAssetsDirectory)package-lock.json;$(ClientAssetsDirectory)package.json</ClientAssetsRestoreInputs>
<ClientAssetsRestoreOutputs Condition="'$(ClientAssetsRestoreOutputs)' == ''">$(ClientAssetsDirectory)node_modules;package-lock.json</ClientAssetsRestoreOutputs>
<ClientAssetsRestoreCommand Condition="'$(ClientAssetsRestoreCommand)' == ''">npm install</ClientAssetsRestoreCommand>
<ClientAssetsBuildCommand Condition="'$(ClientAssetsBuildCommand)' == ''">npm run build</ClientAssetsBuildCommand>
<ClientAssetsBuildOutputParameter Condition="'$(ClientAssetsBuildOutputParameter)' == ''">--outDir</ClientAssetsBuildOutputParameter>
<ClientAssetsRestoreInputs>$(MSBuildProjectFile);$(ClientAssetsRestoreInputs)</ClientAssetsRestoreInputs>
<ClientAssetsOutputPath>wwwroot/App_Plugins/MyLib/</ClientAssetsOutputPath>
</PropertyGroup>
<ItemGroup>
<ClientAssetsInputs Include="$(ClientAssetsDirectory)**" Exclude="$(DefaultItemExcludes)" />
</ItemGroup>
<Target Name="NpmInstall" Inputs="$(ClientAssetsRestoreInputs)" Outputs="$(ClientAssetsDirectory)package-lock.json;$(ClientAssetsDirectory)node_modules/.install-stamp">
<Message Importance="high" Text="Running $(ClientAssetsRestoreCommand)..." />
<Exec Command="$(ClientAssetsRestoreCommand)" WorkingDirectory="$(ClientAssetsDirectory)" />
<Touch Files="$(ClientAssetsDirectory)node_modules/.install-stamp" AlwaysCreate="true" />
</Target>
<Target Name="NpmRunBuild" DependsOnTargets="NpmInstall" BeforeTargets="AssignTargetPaths" Inputs="@(ClientAssetsInputs)" Outputs="$(IntermediateOutputPath)client/assets/build.complete.txt">
<Exec Command="$(ClientAssetsBuildCommand) -- $(ClientAssetsBuildOutputParameter) ../$(ClientAssetsOutputPath)" WorkingDirectory="$(ClientAssetsDirectory)" />
<Message Importance="high" Text="Linking client assets to wwwroot..." />
<ItemGroup>
<_ClientAssetsBuildOutput Include="$(ClientAssetsOutputPath)**" />
<FileWrites Include="@(_ClientAssetsBuildOutput)" />
<FileWrites Include="$(IntermediateOutputPath)client/assets/build.complete.txt" />
</ItemGroup>
<WriteLinesToFile File="$(IntermediateOutputPath)client/assets/build.complete.txt" Lines="@(_ClientAssetsBuildOutput)" />
</Target>
I took this directly from a test project of mine. It writes the file list to build.complete.txt
and checks up against that for any changes the next time you build the .NET project. It will also optionally install npm deps again if the lockfile has changed. Hope it still works :-)
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.
Yeh I am in two minds about dotnet doing magical builds of client side assets and running node/npm stuff
As its kinda voodoo magic that magically just builds it from Rider, VS or dotnet CLI
It doesn't help teach how the client-side assets are built and put into the RCL, but if people have to explicitly run node/npm run commands then they are more conscious of what is going on and how its put together.
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.
Curious to hear from peeps in the team who are building/implementing stuff such as
@nathanwoulfe @mattbrailsford @leekelleher @ronaldbarendse in packages
Copies the tsconfig from the default scaffolding of vite lit-ts CLI
…o what its used for
Summary
umbraco-package
toumbraco-extension
This PR is to rework & reopen the work that @leekelleher done from the previous years community teams day
#14975
Install Command
dotnet new install umbraco-extension --name TestingProject
Example usage