nodejs-storage
nodejs-storage copied to clipboard
Add typing for `File.metadata`
Environment details
- OS: Windows
- Node.js version: 12.20.0
- npm version: 6.14.8
-
@google-cloud/storage
version: 5.18.2
Steps to reproduce
- Create instance of the
File
class. - Access its
metadata
property in TypeScript.
The issue is that its type is effectively … any
, whereas the reference and example demonstrate the structure of the metadata.
Why not to add this structure to the typing?
Hi @anantakrishna ,
This is done on purpose. We don't believe it is good practice to validate the data returned by the server. The way this is currently implemented, the data returned by the server gets propagated to you without being processed in any way. This is safer from bugs on our end because if Cloud Storage changes/adds metadata fields, consumers of this library will get those changes without the current version of the library breaking and/or having to upgrade versions.
Thank you @shaffeeullah for explanation. I partly agree with this logic, however not fully. Let me explain further.
- Now metadata contains such properties as
md5Hash
,cacheControl
and other standard metadata keys. - Some of them are listed in the metadata docs, and more thoroughly they are listed in the JSON API reference.
- The fact that they are listed in the reference seems to mean that the Google Cloud Storage product is committed to their naming and presence.
- This in turn means that the consumer software can rely on them in the source code, which happens de-facto, I believe.
- If and when Storage changes the metadata fields, be it removing or renaming, it will break the consumer software regardless of whether the
nodejs-storage
library has typing support for these metadata keys or not. - Adding new metadata keys is not a problem, the typing can allow unmentioned properties in the interface.
- Typing on itself does not do any transformation of the data, therefore the fact that “data returned by the server gets propagated to you without being processed in any way” is not a concern here.
- The main purpose of adding well-known metadata keys as properties of the
Metadata
type is to facilitate discoverability and type checking. Currently, when one tries to dig into theFile.metadata
propery, they face ambiguousany
and have to research somewhere else on what are the exact metadata keys there.
I hope now the background of my inquiry is clearer.
Hi @anantakrishna while I agree that these are strong advantages of including typing for Metadata
, I still believe that the disadvantages are more impactful.
- If we add typing, users must upgrade the library in order to get new metadata fields instead of getting them automatically
- We don't have a great way of supporting public preview in this library. When GCS metadata features are in public preview, adding those fields to this library potentially leads us to tech debt later (we can't remove fields in this library because that would be a breaking change.
- This feature request in itself is a breaking change.
@shaffeeullah is this still the case if you use fallback index types?
Example: https://www.typescriptlang.org/play?#code/PTAEBcE8AcFMFoDOBDAZrUrkBtsCNkBjAa1AHcBLcAC1AFcA7YhgezIYhlgCgKHxYAJyyEMAWVjhkAE2RSA6lWoBVJq3YAxHPiKkA3t1CYKsbNICMALlAM6AWzxDQAHxt1ch46ekAma4nBBPgBzFzcPIwBtFhohAGlYSER-QJCAXWtGZjYGAG5uAF9ubkIWBgDQO0kZOWQAUQAPZDtobFhrCSlZBSVVbM1tAhJQAF5QAyNUEzMrUABWABpPKe8-UAByRBYqmhD1paNpFlhEVnA7OQFBMmo5AEkABTpwa0XPZAZIXYZg4OPk8LYA6gQh0ALbMFCaSwKYMWDSFZmazrPDYZDUfaFbg7GpSRrNVqwAB0iIsuVAIFAt0QIJYgkEsEI4AWoEQcEIFCmhE4cGx1W69SaLTaJOmvnJlOptPpjOZrPZnIo3KgvJxAvxwtgkXWpJ86zSErAUtKMqZLLZjMVyq4fK6tQ1hKJH0gMWoQlJhqpyBpWXUHBVGDwz1ARxONhioCEgjpLMhghBHxDLB5GEIbpINPAyYYyHpbF4qFAAAoAyxC2r7ULHc7Xe6xaMRmNNqkfusAJTjTySlgANycFbxVZFNdiwnrjmwbBZVE2oH6-uTjlAyFZLdC6lAfAg1AoNMQpTgAEJPAPBQTh59a2PvES2dgqEX1hs2-kisVgAAqL-fn+-n+gK85z4aQAMLLBcCGUhKBoZdPlAD9gGKd8wFkH4hBYMFMEGXRyCUWDIBTIleH4d0iHEflakUGgAEFPi0CCcImLwZmsWwHCcVxbAiZjfBSIIfjCLjsE8aJRwSJI+PSaxnVfYpSnKcBKgowdz1gNZOlxZAqOoWjIHonRhjGJjSVmN5JjFNZNm2SQd1bYFQ1OGILnAK4bnuJ4XnmYFnW+X5-lY9wgU8UFwTsONoVheFSWRVF0UxAp8lPB02h8UVvHMT1jTpBkzXlS0uRTW1NOStS0rMHxMu9aUcrlC0OQKgMivVIc1O1XV9UqmkTRq80FQam0kpa1KRzda8zE6-CU1gkCHPDRSoxjehECcQhEyOKa00ZYhM2zXNozIJrK1U7URqEDQxQ6ikjSq50po+Gb-jmyM80EWNlvjVaOHWgMQXTbaIF2vMDu4IA
Hi @fmmoret ,
This is a fair suggestion. We will consider implementing this. It isn't a breaking change and will allow users to access new GCS metadata fields without upgrading the library.
For my part, being able to access the generation number of the file has value, and since generation number is housed under metadata, it would be nice to have a type contract, or to have generation number live elsewhere even.
Fixed via https://github.com/googleapis/nodejs-storage/pull/2234