Skip to content

Allow to export types easily #22

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

Merged
merged 1 commit into from
Mar 15, 2021
Merged

Conversation

picnoir
Copy link
Contributor

@picnoir picnoir commented Mar 9, 2021

Hi!

I'd need to generate some types for a typescript module, however, there's currently no way to export the generated types.

#17, added this feature, however, the PR seems to have stalled by now. This PR is just a cherry pick of 13822f0 rebased on the current master. I took the liberty to add a tiny bit of documentation.

Allow to export types easily

In some cases, we'd like to use the generated types in a typescript
module instead of a declaration file. When used in a module, the types
need to be explicitely exported to be re-used elsewhere.

Adding a new exportTypes FormattingOption in charge of prefixing the
generated types with "export".

@@ -98,13 +98,16 @@ data FormattingOptions = FormattingOptions
-- ^ Function applied to generated interface names
, typeNameModifier :: String -> String
-- ^ Function applied to generated type names
, exportTypes :: Bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general this PR sounds good to me. I'm wondering if we should reduce boolean blindness / provide for future extensibility by making this data type though. Are there any other kinds of exporting someone might want to do? I know that there's some fancy stuff here like export default, export =, export as namespace... none of them seem very likely to be desired but it would be good to be safe.

Copy link
Contributor Author

@picnoir picnoir Mar 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that was fast :) Thanks a lot for the quick review.

  • export default => I personally don't have a use case for this, but it might be useful in some cases. Introducing a "default type" might require a bit more work API-wise. I guess we should expose a way for the user to tell which Name is supposed to be the default one. I'm not really willing to implement this for now.
  • export as namespace => Could be used to nest the "module name". ie module.namespace.type instead of module.type. It might be useful in some niche cases, I personally don't have any use for this, I'm not sure whether or not we should implement that. I'll let your be the judge here.
  • export = TYPE => It's basically a synonym of export TYPE = (...)

I'm wondering if we should reduce boolean blindness / provide for future extensibility by making this data type though.

I'm not sure I understand what you mean here. Something the lines of:

newtype ExportTypes = ExportTypes Bool
newtype ExportInterfaces = ExportInterfaces Bool
data ExportOptions = ExportOptions ExportTypes ExportInterfaces

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just thinking something like

data ExportMode = 
  ExportEach
  -- ^ Prefix every declaration with the "export" keyword (suitable for putting in a TypeScripe module)
  | ExportNone
  -- ^ No exporting (suitable for putting in a .d.ts file)

that way we can add more constructors if we want to someday

@picnoir
Copy link
Contributor Author

picnoir commented Mar 15, 2021

My apologies for the delay, I missed the notification.

data FormattingOptions = FormattingOptions
{ numIndentSpaces :: Int
-- ^ How many spaces to indent TypeScript blocks
, interfaceNameModifier :: String -> String
-- ^ Function applied to generated interface names
, typeNameModifier :: String -> String
-- ^ Function applied to generated type names
, exportTypes :: ExportMode
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, could you just rename this to exportMode? Otherwise LGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my bad!

@picnoir picnoir force-pushed the nin-export branch 2 times, most recently from 6778282 to 336ccc8 Compare March 15, 2021 16:39
#{ls}
}|] where ls = T.intercalate "\n" $ fmap T.pack [(replicate numIndentSpaces ' ') <> formatTSField member <> ";"| member <- members]
modifiedInterfaceName = (\(li, name) -> li <> interfaceNameModifier name) . splitAt 1 $ interfaceName

formatTSDeclaration (FormattingOptions {..}) (TSRawDeclaration text) = text

exportPrefix :: ExportMode -> String
exportPrefix em =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh one more nit, sorry -- could you convert this case statement to a pattern match? i.e.

exportPrefix ExportEach = "export "
exportPrefix ExportNone = ""

In some cases, we'd like to use the generated types in a typescript
module instead of a declaration file. When used in a module, the types
need to be explicitely exported to be re-used elsewhere.

Adding a new exportTypes FormattingOption in charge of prefixing the
generated types with "export".
@thomasjm
Copy link
Contributor

Perfect, thanks! Merging now

@thomasjm thomasjm merged commit 8fd3a71 into codedownio:master Mar 15, 2021
@picnoir picnoir deleted the nin-export branch April 13, 2021 19:48
nh2 added a commit to nh2/aeson-typescript that referenced this pull request Oct 4, 2021
No point having a default when the user cannot access to override it!

This was forgotten in PR codedownio#22 when extracting it from PR codedownio#17.
nh2 added a commit to nh2/solga that referenced this pull request Oct 4, 2021
…ixes.

Our fixes: codedownio/aeson-typescript#17

The one that wasn't taken verbatim, which this commit adapts to:

Upstremed in: codedownio/aeson-typescript#22

We still have to use our fork to fix `defaultFormattingOptions` not being
exported:

codedownio/aeson-typescript#27
thomasjm pushed a commit that referenced this pull request Oct 4, 2021
No point having a default when the user cannot access to override it!

This was forgotten in PR #22 when extracting it from PR #17.
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

Successfully merging this pull request may close these issues.

3 participants