feat: add ESM path to generator #1402
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ddelgrosso1, these are the changes for the generator. This is failing because it's attempting to run |
Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com>
* fix: correct IAM return types * tests: update baseline * build: remove kms test until it can be updated for gax
Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com>
Bumps [word-wrap](https://github.com/jonschlinkert/word-wrap) from 1.2.3 to 1.2.4. - [Release notes](https://github.com/jonschlinkert/word-wrap/releases) - [Commits](jonschlinkert/word-wrap@1.2.3...1.2.4) --- updated-dependencies: - dependency-name: word-wrap dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [c8](https://github.com/bcoe/c8) | [`^8.0.0` -> `^8.0.1`](https://renovatebot.com/diffs/npm/c8/8.0.0/8.0.1) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>bcoe/c8 (c8)</summary> ### [`v8.0.1`](https://github.com/bcoe/c8/blob/HEAD/CHANGELOG.md#801-2023-07-25) [Compare Source](https://github.com/bcoe/c8/compare/v8.0.0...v8.0.1) ##### Bug Fixes - **deps:** update istanbul deps ([#​485](https://github.com/bcoe/c8/issues/485)) ([3be8bd9](https://github.com/bcoe/c8/commit/3be8bd97c3fb38b575a48871c170dd5e5041b9a8)), closes [#​478](https://github.com/bcoe/c8/issues/478) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/googleapis/gapic-generator-typescript). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMS4wIiwidXBkYXRlZEluVmVyIjoiMzYuMTEuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
…ebpack devDependencies (#1416) [](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [gapic-tools](https://github.com/googleapis/gax-nodejs) | [`^0.1.7` -> `^0.1.8`](https://renovatebot.com/diffs/npm/gapic-tools/0.1.7/0.1.8) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/googleapis/gapic-generator-typescript). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4yNC4yIiwidXBkYXRlZEluVmVyIjoiMzYuMjQuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->
sofisl
changed the title
WORKSPACE
Outdated
| @@ -40,4 +40,4 @@ npm_translate_lock( | |||
|
|
|||
| load("@npm//:repositories.bzl", "npm_repositories") | |||
| npm_repositories() | |||
| pnpm_repository(name = "pnpm") | |||
| pnpm_repository(name = "pnpm") | |||
](https://renovatebot.com){kind=link}
There was a problem hiding this comment.
I believe this file can be reverted since the only change is the linebreak.
baselines/asset/package.json
Outdated
| "jsdoc-fresh": "^3.0.0", | ||
| "jsdoc-region-tag": "^2.0.1", |
There was a problem hiding this comment.
Why the downgrade?
| @@ -1,7 +1,5 @@ | |||
| import * as $protobuf from "protobufjs"; | |||
|
|
|||
| // Note: this file was manually edited to remove references to Long which are not needed in this project. | |||
There was a problem hiding this comment.
I think it's a good idea to remove references to long.
typescript/src/schema/proto.ts
Outdated
| @@ -26,7 +26,17 @@ import {BundleConfig} from '../bundle.js'; | |||
| import {Options} from './naming.js'; | |||
| import {ServiceYaml} from '../serviceyaml.js'; | |||
| import protobuf from 'protobufjs'; | |||
| import protoJson from '../../../protos/protos.json' assert {type: 'json'}; | |||
There was a problem hiding this comment.
Why is this change needed? What's wrong with the regular import? The path is static anyway, isn't it?
typescript/test/unit/util.ts
Outdated
| @@ -25,6 +25,11 @@ import protobuf from 'protobufjs'; | |||
| import protoJson from '../../../protos/protos.json' assert {type: 'json'}; | |||
| import type * as protos from '../../../protos/index.js'; | |||
|
|
|||
| import * as fs from 'fs'; | |||
There was a problem hiding this comment.
Please remove unused imports.
typescript/test/util.ts
Outdated
| if (options.diregapic) { | ||
| commandLine += ' --diregapic'; | ||
| } | ||
| if (options.mixins) { | ||
| commandLine += ` --mixins="${options.mixins}"`; | ||
| } | ||
| console.log('COMMANDLINE ARG:'); |
There was a problem hiding this comment.
Please remove the extra logging.
typescript/tools/update-baselines.ts
Outdated
| try { | ||
| await fsp.rm(baselineDir, {recursive: true}); | ||
| } catch (err) { | ||
| console.log('no baseline to delete'); |
There was a problem hiding this comment.
If we just ignore this, let's ignore silently. Otherwise, this log messages is not very helpful, should've said "Not removing baseline ${baselineDir} because it does not exist", or something like that.
alexander-fenster
approved these changes
Sep 25, 2023
…escript into moveToEsm
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a path to generate ESM-templates in the generator by adding a
formatcommand-line variable. If omitted, it will generate (regular) cjs code. If included, it will generate ESM-code, that transpiles into dual format. Templates now live undercjsandesmdirectories.