Umbraco-cms: Remove AngularJS directive's template files from the compiled build

[PeteDuncanson]

Now that directive templates are stored inline as part of the build (see #4845) we don't need to include the original template files in the zipped/nuget builds.

Ideally these should/could be removed to save some bytes in the output zip file.

Kicked off by this chat on twitter: https://twitter.com/nathanwoulfe/status/1154711340323201025

[nul800sebastiaan]

Yep, @dawoe also pointed that one out! Would indeed be good to remove them from the build output as they're no longer necessary! 👍

[umbrabot]

Hi @PeteDuncanson,

We're writing to let you know that we've added the Up For Grabs label to your issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly PR team bot :-)

[nathanwoulfe]

Not sure how this would work given the lack of structure in the backoffice project - all the view files are co-located, so it's going to be a challenge to identify directive templates vs controller templates. Most directive views are in /views/components/**, but not all.

I have changes in #5785 to add the view path as an attribute on the root element for a given template, which would be a possible clash (or solution) to this issue. I've changed the order of the gulp task execution as I needed the final view path to add into the file, so views are copied before the js task to embed the template in the directive, which in turn means the Gulp task is reaching outside the Web.UI.Client project, which isn't great.

I can update that PR to add a check for the ng-controller attribute on the root element, to identify a controller view, then only copy those files in the subsequent pipe() task. Except that will fail because the embedTemplates task runs after the views are copied, so needs everything.

So yeah. It's a good idea, just not sure how to implement it. Will have a look into it though.

[nathanwoulfe]

As a proof-of-concept, I can rename the directive views (ie filename.temp) when copying, then add a cleanup task to rip back through the folders and delete *.temp.

[nathanwoulfe]

I can get this working, but have been thinking - if I'm adding the path in #5785, then removing the view in this task, does that make sense? Does it even make sense to inline the view path for directives, when the template itself will be inlined into the JS? Would it make sense to only add the view path for controller templates, since those files will remain on disk? What about managing it all differently when building from source versus running a compiled build? So many questions?

ping @PeteDuncanson since it's your fault my brain is now going in circles... @dawoe and @nul800sebastiaan because they shouldn't miss out.

[Shazwazza]

Hi all - There is now a development issue with the inlining of these templates. Now when we are developing and we change a view with the file watch enabled, we don't get the updated view.

I've fixed this in the gulp build here 9d77114d5038b6403f78d67202f5f8f60c383afa

• don't inline the templates if not in isProd build
• removes fastdev, only Niels L used this but we don't need it, the normal dev build is now also fast
• ensures that eslint always runs for dev builds

[nul800sebastiaan]

Yikes. Sounds like a small thing until it suddenly explodes! 🙈

Let's reset:

Problem: view files are copied to ~/src/Umbraco.Web.UI/Umbraco/views but they should not be because they're being inlined with the js files.

Given that fact: is there any reason ~/src/Umbraco.Web.UI/Umbraco/views needs to exist at all any more?
I guess there is, since I am ignorant about what "directive templates" means, it seems to indicate other templates are still necessary.

So if only a subset needs to not be there can we (instead of deleting them after the fact) just not copy them?

I see @nathanwoulfe is adding another list of gulp dependencies now so we can delete files, these dependencies need to also be kept up to date all the time and that is already proving to be difficult at the moment. So I'd much rather avoid doing things that we need to clean up after the fact.

Please enlighten me if I got this all wrong though! 😁

[PeteDuncanson]

Good spot @Shazwazza and a good fix.

@nul800sebastiaan dependancies on their own aren't an issue, old abandoned ones are though (like the docs task). But NPM makes updating any others pretty easy so I wouldn't be worried about adding those if it helps us make Umbraco better/faster. It is good that we don't just add anything we fancy and that there are some over watch on what gets added.

I believe @nathanwoulfe mod will only copy templates over if they aren't inlined (not everything is/can be inlined). So I think everything as is is as it should be now.

[nul800sebastiaan]

The reason I'm worried is because we keep running into "oh, this doesn't work any more.. Ah, that's because we need to update 43 dependencies now, oops, this module doesn't work with this version of that module, we need to make a dependency a fixed version.. oh but now we can't update module 'xyz' because...".

I know, we're probably doing it all wrong but you know: once bitten, twice shy (thrice bitten...).

Anyway, happy for whatever is needed as long as we're sure it's really needed. 👍

[nathanwoulfe]

My issue was around the combination of this change and adding the view path into the markup - if the markup references a file which has been removed because it is inlined in the Javascript, is it still useful to have the view path referenced?

I think this relates to Shannon's change - what should happen in a dev vs production build? Inlining views for prod, definitely, deleting filed where view is inlined, definitely, but adding the file path is probably only useful in dev since that's when the referenced files will still exist. I'm not sure.

That said, I'm using the presence of the inlined template as the flag for deletion - find them all in the compiled Javascript, the get the file path from the inlined template string, then delete that file. If it's not referenced in a template property in Javascript, the file isn't touched.

As for npm, so long as dependencies are reviewed every so often, and not allowed to get super stale, there shouldn't be any issues. Still waaaay more efficient than writing everything yourself!