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

Allow templates to omit App_Resources #2513

Closed
tjvantoll opened this issue Feb 8, 2017 · 11 comments
Closed

Allow templates to omit App_Resources #2513

tjvantoll opened this issue Feb 8, 2017 · 11 comments
Assignees
Labels
Milestone

Comments

@tjvantoll
Copy link
Contributor

As the NativeScript 2.5 release, NativeScript templates now must provide a complete App_Resources folder in order to work. I believe we should revert that change, and make App_Resources optional for templates. Here’s why:

  • The easier we make templates to create the more people will create them. We don’t have many templates right now and it’d be great to have more community-written ones.
  • Very few templates will actually need to change files in App_Resources. Few templates will need to add images, and none will want to alter the default splash screens or icons. That means most template authors will do what I did, and copy/paste the App_Resources folder from the default hello world template.
  • We update the App_Resources folder in the default template fairly often, and I want those updates to make it into my templates. To accomplish this currently I have to manually check for App_Resources updates with each release, copy and paste the folder again, and publish a brand new version of each of my templates.

I realize that this change was made for performance reasons—that we don’t want users to download two templates when using App_Resources-free templates. But I believe the developer convenience outweighs the moderate performance gains. And most users will continue to use one of our three default templates, all of which include an App_Resources folder, and will therefore not be subject to this small performance hit.

@nraboy
Copy link

nraboy commented Feb 8, 2017

Thanks!

@Plamen5kov
Copy link
Contributor

@nraboy @tjvantoll The performance hit wasn't the big problem which led to the decision. The actual problem was, the behavior we had was error prone, because CLI augmented the App_Resources folder when it was present, but it didn't contain all the necessary files. The second reason is the templates can differ and while the copy/paste solution might work in some cases, an angular template can differ a lot from a vanilla js template, and we might tie our hands on using more than we need. In third place is the performance issue (additional traffic).

As @tjvantoll said:

That means most template authors will do what I did, and copy/paste the App_Resources folder from the default hello world template.

One possible solution for developers making templates to add the App_Resources folder in their templates and publish them.


We update the App_Resources folder in the default template fairly often, and I want those updates to make it into my templates.

That's exactly right, the templates are updated often, but what happens when the angular templates App_Resources is different from the vanilla one. What should the "default" one be. We'll be making decisions instead of the template author, which has proven error prone.

@tjvantoll
Copy link
Contributor Author

The actual problem was, the behavior we had was error prone, because CLI augmented the App_Resources folder when it was present, but it didn't contain all the necessary files.

This I agree with. I only believe the CLI should add an App_Resources folder if the template doesn’t contain that folder at all. Augmenting the folder could definitely be error prone.

[T]he templates are updated often, but what happens when the angular templates App_Resources is different from the vanilla one. What should the "default" one be. We'll be making decisions instead of the template author, which has proven error prone.

I just did a quick compare using diff -rq, and only difference between https://github.com/NativeScript/template-hello-world-ng/tree/master/App_Resources and https://github.com/NativeScript/template-hello-world/tree/master/App_Resources are comments in two files. Functionality wise they’re identical.

It’s not clear to me why App_Resources would be different between Angular and non-Angular apps.

@rosen-vladimirov
Copy link
Contributor

@tjvantoll , @Plamen5kov
App_Resources should have really specific names, at least in the templates. The previous logic in CLI was using App_Resources from default template only for the files that do not exist in the currently used on.
For example in case the ng template needs to change only one icon in the App_Resources, during tns create app --ng, CLI was installing the template, comparing the App_Resources from the current template with the ones from the default template and copying only the ones that are missing. The idea was that when new iPhone requires additional icon, we wouldn't have to update all of the templates, just add it to the default template and everything would work. In case any template needs to change all resources, just add them and CLI wouldn't modify them during creation.
That was the behavior in the previous versions ;)

@Plamen5kov
Copy link
Contributor

Including @tjvantoll since he was involved the first time I reported something like this. I personally don't think either of the proposed solutions are good solutions.

@nraboy, please give us your suggestions, so we can continue the discussion from #2493, so we can reach a solution together, hopefully for 3.0.

@nraboy
Copy link

nraboy commented Mar 2, 2017

My suggestion is to have the CLI check to see if the App_Resources directory exists during creation and if it does not, create it.

I can't imagine this being difficult to do.

I think it is critical that people be able to specify template versions.

@tjvantoll
Copy link
Contributor Author

tjvantoll commented Mar 2, 2017

I’ll +1 @nraboy.

if (the App_Resources directory exists) {
    // do nothing
} else {
    // add the App_Resources directory from the default template
}

This simple change would make templates easier to build and maintain.

@nraboy
Copy link

nraboy commented Mar 3, 2017

@Plamen5kov what needs to be done to make what @tjvantoll had suggested happen?

@Plamen5kov
Copy link
Contributor

Plamen5kov commented Mar 6, 2017

@nraboy If we don't want to support anything different than what @tjvantoll suggested, it should be no problem doing this for 3.0 release. I'm all for your solution, as long as it's clear that it's error prone, with cli using templates that are created by developers that don't know about the App_Resources folder and make templates that would be incompatible with the default App_Resources. The other thing is the additional traffic of the default template, that will have to be downloaded at least once per machine always, and every time npm cache clean is called

@Plamen5kov
Copy link
Contributor

Plamen5kov commented Mar 6, 2017

Further more @tjvantoll @nraboy when would you expect CLI to make that detection, only on tns create or on every command you run?

@nraboy
Copy link

nraboy commented Mar 6, 2017

I would only expect this at creation. If the user messes up their project later, it is on them.

@Plamen5kov Plamen5kov added this to the 3.0.0-RC milestone Mar 10, 2017
@yyosifov yyosifov self-assigned this Mar 13, 2017
dtopuzov added a commit to NativeScript/nativescript-cli-tests that referenced this issue Apr 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants