skill-sample-nodejs-hello-world icon indicating copy to clipboard operation
skill-sample-nodejs-hello-world copied to clipboard

Unnecessary code for a "hello world" skill?

Open habuma opened this issue 5 years ago • 3 comments

In recent months, I've found that some of the latest changes to this skill template are (at least in my mind) questionable. I've looked at the "Hello World" template as a barebones baseline skill project that can be used as the basis for almost any skill I might write. With that in mind, I'm wondering if there's opportunity for pruning (or if not, then an explanation of why some things are included):

  • The call to withCustomUserAgent() in index.js : What purpose does this serve? I know it affects the "userAgent" property in the response, but I find very little documentation or explanation around how this is useful.
  • Interaction models for ALL languages and locales : I'm all for supporting multiple languages and locales, but I find it easier to start with one language/locale and expand to others once the functionality of the skill has matured. Otherwise, anytime I create a new intent, I must make sure I declare it in all interaction model JSON files. It's much easier to start with one language/locale and then, when ready, copy that file for others and tweak it accordingly. Also, I've found that initially having models for all languages/locales makes the model compilation unnecessarily slower. Moreover, even if I plan to support multiple languages and locales, I might not be planning to support all of them, so having those extra model files around is waste and forces me to delete them.
  • Similarly, are all of the languages/locales in skill.json really necessary when getting started? I'd rather grow into those languages/locales as my skill matures than to have to think about them on day one.
  • lambda/custom/util.js : I can see how this module would be useful if my SSML responses need to reference a sound file stored in S3 or I'm building some APL that reference an image stored in S3. But the hello world template does neither of these. And, in fact, I can develop a fairly fully-functioning skill and never need util.js. Therefore, it seems to be dead weight in this project. Moreover, if it is potentially useful, then I'd rather it be baked into ASK or be a separate module that I could npm install, rather than be duplicated across all skill projects.
  • local-debugger.js : Now, I must say that I LOVE this little script and use it all of the time. But, should it be sitting in lambda/custom alongside my skill code? Could it not live in a separate folder so that it doesn't get deployed to AWS Lambda with my skill? (Yes, I cracked open the ZIP file that's uploaded and it's in there.)
  • "AMAZON.NavigateHomeIntent" in the models : This template in no other way demonstrates creating visual components for screen-enabled devices, so why is this intent even declared in the model(s) for this template?
  • (Small nit) I'd rearrange the intent declarations in the models such that all built-in intents are grouped together and stop and cancel are together, with the "HelloWorldIntent" declared either first or last (I'd favor first, but I'd be okay with it either way).

Again, I view the "hello world" template as the simplest possible example of a skill and some of the stuff I've mentioned strays from that. When cards were removed from the template, I initially was upset to see them go, but then justified their removal because they were unnecessary in the "simplest possible skill", so I'm fine with them being gone. But these other items I've listed above seem counter to the "simplest possible skill".

I'd be happy to do a little work and produce a PR to address these items, but given that these are mostly due to recent changes, I'd imagine that there might be a reason for them and I'd love to hear the reasoning behind these things.

habuma avatar Dec 08 '19 03:12 habuma

All that said, I do like the addition of languageStrings.js and the request interceptor that adds the t() function to handlerInput. Even in a simple skill, externalizing strings like that puts the developer on the right path toward later supporting multiple languages. I'd just trim languageStrings.js down to one language (maybe at most two locales in the models and skill.json) and invite the developer to add additional languages as they see fit.

habuma avatar Dec 08 '19 04:12 habuma

FWIW, given silence on this issue for over a week, I've created my own simplified "hello world" template at https://github.com/habuma/skill-template-nodejs-hello-world. If the official template isn't likely to be simplified, then I invite anyone who is interested to both use it and provide feedback.

habuma avatar Dec 16 '19 16:12 habuma

Hey Habuma, I agree with a lot of this. To address the complexity problem, there are now separate branches for each locale (named after the locale) with simplified code. This will be used for Alexa hosted templates. We will be keeping the localization in other skill samples, but for hello world, this should be as simple as possible.

I am leaving the main branch as is in case others are relying on localization code being there for now until.

JoeMoCode avatar Aug 11 '20 00:08 JoeMoCode

Closing because we created different branch.

aszk avatar Apr 07 '23 18:04 aszk