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

Improve TS compatibility checks #296

Merged
merged 38 commits into from
Nov 11, 2022
Merged

Improve TS compatibility checks #296

merged 38 commits into from
Nov 11, 2022

Conversation

smaye81
Copy link
Member

@smaye81 smaye81 commented Nov 10, 2022

This makes the TS compatibility checks a bit more robust by fixing a few issues in the codebase as well as changing our TypeScript test configs to include all files rather than just the test files.

Note that the long package needed downgraded to 5.0.1 (and pinned to patches only) because of an issue in that library with 5.2 and above. See this issue: dcodeIO/long.js#109

Massaged the types a bit so that we can actually compile all test sources now, down to TS v4.4.4.

There are some oddities with the package "long" that we need to investigate.

We also need to continue this work further down to older TS versions.
Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

Thanks for adding more files to the compilation! Two nits though:

// to ignore them here and still get version compatibility assurance.
"../src/gen/ts/google/protobuf/*.ts",
"../src/google/protobuf/*.test.ts",
"../src/message.test.ts"
Copy link
Member

Choose a reason for hiding this comment

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

We should be able to include message.test.ts, if we change the test to use Timestamp from @bufbuild/protobuf instead of the locally generated file.

// to ignore them here and still get version compatibility assurance.
"../src/gen/ts/google/protobuf/*.ts",
"../src/google/protobuf/*.test.ts",
"../src/message.test.ts"
Copy link
Member

Choose a reason for hiding this comment

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

Same here, should be able to include message.test.ts.

packages/protobuf-test/typescript/tsconfig.4_1_2.json Outdated Show resolved Hide resolved
"include": ["../src/**/*"],
"exclude": [
// Exclude WKTs and their usage in tests because they use syntax not
// compatible with older versions of TypeScript. WKTs used by actual
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

smaye81 and others added 27 commits November 11, 2022 11:20
This adds a new package `protoplugin-example` showing how to create a
plugin that generates a Twirp client using the plugin framework. The
client can then issue requests to our Connect demo server to illustrate
it is working and is also compatible with our Connect server.

A few things to note:

- Since Twirp does not support streaming, all streaming methods in the
generated clients will throw an error saying that streaming is not
supported.
- This does not have true Twirp error handling as this is just a basic
example for writing a plugin.
- Related to the above, since this is meant to be a basic example, the
generated client only support JSON request bodies.

Co-authored-by: Timo Stamm <ts@timostamm.de>
There are conditions where TypeScript files must be generated even if
`ts` was not specified in the target out. This is because the TypeScript
files are needed for the transpilation process.

In these circumstances, the plugin should not generate actual `.ts`
files. This removes them from the `CodeGeneratorResponse`.
This adds an extensive README to `protoplugin` which illustrates how to
create plugins using the plugin framework.
This release includes the following:

## 🚨 Breaking Changes 🚨 
- #233 - If you have been inspecting fields (`DescField`) from
createDescriptorSet(), you will need to update your code to use the new
`fieldKind` discriminator. The `kind` property no longer contains values
such as `map_field` and `message_field` for their respective types of
`DescField`. Instead, a new type has been added named `fieldKind`, which
allows you to further identify the kind of field a `DescField`
represents (`map`, `enum`, `message`, etc.)

- #228 - If you have been using `createEcmaScriptPlugin` to create your
own plugin, the signature has changed. Previously, the function accepted
a single generator function for generating your output files. This has
been split up into separate functions for TypeScript (`generateTs`),
JavaScript (`generateJs`), and declaration files (`generateDts`). See
the [plugin
docs](https://github.com/bufbuild/protobuf-es/blob/main/docs/writing_plugins.md)
for more information on usage.

## Enhancements
* Add plugin example for creating a Twirp client by @smaye81 #250
* Export printable type by @fubhy #255
* Extend documentation for PlainMessage and PartialMessage by @timostamm
#248
* Add convenience functions for retrieving custom options by @smaye81
#244
* Clean up descriptor discrimination by @timostamm #233
* Add a plugin option to keep empty files by @timostamm #229
* Add the option to transpile target files by @smaye81 #228

## Bugfixes
* Fix TypeScript file generation bug by @smaye81 #253
* Fix: arbitrary imports by @fubhy #254

## New Contributors
@fubhy made their first contributions in #254 and #255.
Exposes the WKT helper method previously known as `matchWkt`. This
method helps to transform a general `DescMessage` into a more concrete
`DescWkt` type, which allows for easier access to the WKT's fields.
Useful during code generation functionality.
This updates the custom option unit test for a float value. Due to
#280, it was realized that
the parsed value of a float will not always be equal to the exact value
set in the proto file. Therefore, we need to compare to the nearest
32-bit single precision value.
```
npm install --prefix packages/protobuf-bench esbuild@latest
npm install --prefix packages/protoplugin-example tsx@latest
npm install eslint@latest
npm install @typescript-eslint/eslint-plugin@latest
npm install --save-dev @typescript-eslint/parser@latest
npm install --save-dev eslint-import-resolver-typescript@latest
npm install jest@latest
npm install @types/jest@latest
```

Co-authored-by: Steve Ayers <sayers@buf.build>
This enhances the `Any.is` function to allow for passing a `typeName` as
either a `MessageType` or a string representation of a typeName.
yukukotani and others added 9 commits November 11, 2022 11:21
Adds `GeneratedFile#printTag` to improve the human-readability of
plugins.

Please note that this PR is just a proposal. Also, the naming of
`printTag` is debatable.

## Example

Extracted from
[here](https://github.com/bufbuild/protobuf-es/blob/1d27516e6455675e778a807d8865b1bdf8c6d97a/packages/protoplugin-example/src/protoc-gen-twirp-es.ts#L68)

### Before

```ts
f.print("    async ", localName(method), "(request: ", method.input, "): Promise<", method.output, "> {");
```

### After

```ts
f.printTag`    async ${localName(method)}(request: ${method.input}): Promise<${method.output}>{`
```
Co-authored-by: Timo Stamm <ts@timostamm.de>
#279 adds an overload to
`print` for using a template literal tagged function. This updates the
docs to illustrate that.
@smaye81 smaye81 force-pushed the sayers/improve_ts_compat branch from 91a43b9 to 315b5a8 Compare November 11, 2022 16:27
@smaye81 smaye81 merged commit 6372cde into main Nov 11, 2022
@smaye81 smaye81 deleted the sayers/improve_ts_compat branch November 11, 2022 17:01
@smaye81 smaye81 mentioned this pull request Nov 23, 2022
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.

8 participants