Skip to content
This repository was archived by the owner on Mar 11, 2026. It is now read-only.

fix: stop exporting express types publicly#376

Merged
ofrobots merged 3 commits intogoogleapis:masterfrom
ofrobots:missing-express-types
Feb 4, 2019
Merged

fix: stop exporting express types publicly#376
ofrobots merged 3 commits intogoogleapis:masterfrom
ofrobots:missing-express-types

Conversation

@ofrobots
Copy link
Copy Markdown
Contributor

@ofrobots ofrobots commented Feb 1, 2019

We don't have a dependency on express. If we expose express types
as part of our public, our dependent modules will need to have a dev
dependency on @types/express. Otherwise they see their builds failing.
Currently there is no way to express this dependency relationship to
npm.

Instead, let's not use express types.

Ref: googleapis/nodejs-logging-bunyan#243

We don't have a dependency on express. If we expose express types
as part of our public, our dependent modules will need to have a dev
dependency on @types/express. Otherwise they see their builds failing.
Currently there is no way to express this dependency relationship to
npm.

Instead, let's not use express types.

Ref: googleapis/nodejs-logging-bunyan#243
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 1, 2019
Copy link
Copy Markdown
Contributor

@JustinBeckwith JustinBeckwith left a comment

Choose a reason for hiding this comment

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

After all of that debate.... this is a much, much better approach 👏

@ofrobots
Copy link
Copy Markdown
Contributor Author

ofrobots commented Feb 2, 2019

This doesn't solve the problem generally. It just kicks the can down the road.

@ofrobots
Copy link
Copy Markdown
Contributor Author

ofrobots commented Feb 2, 2019

@DominicKramer @JustinBeckwith do you have opinions on the semver-ness here?

The APIs are supposed to be internal APIs between logging-{winston,bunyan} and this common library. AnnotatedRequestType is removed, but that was not used by the client libraries. The other type changes compile without any changes in the client libraries.

I deem this to be semver-patch. Let me know if you disagree.

@JustinBeckwith
Copy link
Copy Markdown
Contributor

Patch is fine by me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants