feat: Allow optional suffix for generated files #431
Conversation
|
@stephenh Ok, opened the pull request. Please provide feedback. Due to how the code generation handles imports, it was a little more invasive that I was initially expecting. The |
There was a problem hiding this comment.
@sallustfire nice, looks great! Let me know if you want to make the imp helper method update; if not, np, I'm good to merge this as-is.
| @@ -42,6 +43,13 @@ describe('options', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it('can set fileSuffix', () => { | |||
| @@ -721,7 +722,7 @@ function generateDecode(ctx: Context, fullName: string, messageDesc: DescriptorP | |||
| } | |||
| } else if (isEnum(field)) { | |||
| if (options.stringEnums) { | |||
| const fromJson = getEnumMethod(typeMap, field.typeName, 'FromJSON'); | |||
| const fromJson = getEnumMethod(ctx, field.typeName, 'FromJSON'); | |||
There was a problem hiding this comment.
Ah yeah, good call passing in the whole ctx instead of both typeMap + options.
src/encode.ts
Outdated
| const name = wrapperTypeName(typeName); | ||
| if (!name) { | ||
| return code`${messageToTypeName(ctx, typeName, { keepValueType: true })}.encode(value).finish()`; | ||
| } | ||
|
|
||
| if (name == 'Timestamp') { | ||
| const TimestampValue = imp(`${name}@./google/protobuf/timestamp`); | ||
| const TimestampValue = imp(`${name}@./google/protobuf/timestamp${options.fileSuffix}`); |
There was a problem hiding this comment.
Wdyt about making an imp helper method in src/utils that looks something like:
imp(ctx, name, `./google/protobuf/timestamp`);
And then does the stitching together there? Might be nice to centralize the fileSuffix handling.
There was a problem hiding this comment.
@stephenh Ok, I created a helper impProto to consolidate the logic as described, the suffix avoids naming collisions since there are imports of public JS libraries too. All of the imports had template literals with relative paths, so it's really a style call on whether you consider the './' part of the module name or not.
There is also a circular dependency caused by makeUtils if the context is one of the function parameters. Just using the options avoids this.
Anyway, happy to make changes here if you have different preferences.
|
@sallustfire cool, looks great! |
# [1.93.0](v1.92.2...v1.93.0) (2021-12-08) ### Features * Allow optional suffix for generated files ([#431](#431)) ([d826966](d826966))
|
🎉 This PR is included in version 1.93.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
@stephenh Thanks, that was very fast! |
Implements the functionality described in #383 which allows for custom suffixes on generated files, which can be specified via the new plugin option
fileSuffix.