Clearer error messages in read-only properties set attemps

[gioragutt]

Currently, failSet does not expose what property you tried to set.

My suggestion:

const failSet = (prop) => () => { throw new Error(`Cannot set read-only property R2.${prop}`) }
...
Object.defineProperty(this, 'json', {
  get: () => this.response.then(resp => resp.clone().json()),
  set: failSet('json')
})
...

this way, the error that will be thrown will allow for quicker and easier debugging, since you'll more easily know what to search for in your code.

[gr2m]

Can you explain a use case where you would accidentally set one of these properties such as json? I can’t think of why one would ever try to do that in the first place?

[gioragutt]

I forgot about this issue completely :P What I meant is, that json was just an example. There are several read-only fields that are being protected from being set (with failSet), but when such a failure happens, the log message does not indicate what did you try to set. The purposed change would now look like so:

const failSet = way => () => throw new Error('Cannot set read-only property R2.${way}') // <-- see this
const resolveResWith = way => resp => resp.clone()[way]()

/* formData isn't implemented in the shim yet */
const ways = ['json', 'text', 'arrayBuffer', 'blob', 'formData']
ways.forEach(way =>
  Object.defineProperty(this, way, {
    get: () => this.response.then(resolveResWith(way)),
    set: failSet(way) // <--  fail set will tell you what read-only property you tried to change
  })
)

[gr2m]

gotcha, thanks for following up after all this time 👍 I’ll leave this open but I think it will become obsolete as we will move away from the getters and use functions instead in a future version, as discussed in #22

[gioragutt]

@gr2m got it 😄 methods are definitely the better solution over getters+(unwanted)setters.

I'm not using the library now, I don't even remember when I had to use it, but I'm glad that good design decisions are taken nonetheless. Keep it up 👍