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

Increase type safety #6

Open
wolverian opened this issue Apr 18, 2018 · 5 comments
Open

Increase type safety #6

wolverian opened this issue Apr 18, 2018 · 5 comments

Comments

@wolverian
Copy link

Feel free to reject this as something out of cloudform's scope:

It'd be great if cf.Fn.Ref() calls failed to compile if the referenced resource doesn't exist in the template.

I have an example project that deploys an ECS cluster and a service with cloudform: https://github.com/wolverian/ts-ecs-example. From writing it (and from using cloudform in another, private project), one of the biggest pain points I've experienced is making sure I give the correct string values to cf.Fn.Ref() and cf.Fn.GetAtt().

I don't have a concrete suggestion how to implement this, but any ideas are welcome.

@NOtherDev
Copy link
Collaborator

Yes, I've been thinking about a solution for this and although I haven't published anything yet, I have some ideas. I need to weight whether the API complexity added is worth it. I'll elaborate soon.

By now, what I do is to use a dictionary object with all the resource names defined and reference these names through that object only. See const Resource in the example. But yes, this is only a convention, not a type safety measure.

@wolverian
Copy link
Author

wolverian commented Apr 18, 2018 via email

@NOtherDev
Copy link
Collaborator

Yes, although requiring the API user to always specify that MyResources generic parameter would kill the usefulness for me.

cloudform<MyResources>({
  Resources: {
    x: {
      Ref: Fn.Ref<MyResources>('x')
    }
  }
})

I'm now thinking about exposing that type-safe Ref via the typed callback function like this:

const MyResources = {
  x: 'x'
}

cloudform<typeof MyResources>(context: (Context<typeof MyResources>) => ({
   x: {
      Ref: context.Ref('x')
    }
}))

or maybe better by passing this MyResources definition in:

// cloudform<T>(resourcesDefinition: T, templateBuilder: (context: Context<T>) => Template)

cloudform(MyResources, context => ({
   x: {
      Ref: context.Ref('x')
    }
}))

WDYT?

@wolverian
Copy link
Author

LGTM. I think I prefer the latter approach, as it looks simpler. I'll play around with this approach a bit and see how it works. I'll report if I bump into problems. I'd be happy to contribute a PR later, though it's fine if you implement this before I get around to it. 👍

@fhewitt
Copy link
Contributor

fhewitt commented Jun 11, 2018

I strongly recommend to add a linter step (we use cfn-lint) over the CloudForm result for additional validation. It's not perfect, but this can grab not only resource reference issues, but some other errors too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants