fix: 🐛 Cannot set property src of #<OpenStoriesElement> which has only a getter

[JamesIves]

Related: dddddddddzzzz/OpenStories#10

I kept running into the following error intermittently, I thought it was because I was providing src incorrectly but it appears I was wrong. Because of the type of error being thrown here, in certain frameworks such as Next.js it prevents the whole page from rendering.

TypeError: Cannot set property src of #<OpenStoriesElement> which has only a getter

From my understanding of this error, this is due to the usage of the get keyword without an accompanying set keyword. In this pull request, I've refactored src and duration so they implement set which resolves the error for me.

I've validated the best I can but would appreciate it if you pulled this down locally to test everything out too (if you wish to merge this that is) as you all know the ins and outs of this better than I do 😄

[muan]

hey sorry it took me a while to review.

1. getter should always reflect the current content attribute value (in this case full absolute URL for src, number for duration).
2. setter should update the content attribute

for example, with HTML,

<form id action=​"/​" method=​"get">​…​</form>​

Console,

$ form.action
> 'https://github.com/'
$ form.action = '/test'
> '/test'
$ form.action
> 'https://github.com/test'

In the current PR the content attribute is never read. If tested with a plain HTML <open-stories> element, el.src would be empty, thus breaking the test page (/demo/).

[JamesIves]

Ah, good catch! I wonder if I missed this because a React lifecycle was triggering the setter when I was testing. 🤔

Made some refactors by calling getAttribute in the constructor and moving the path conversion into its own method. It should work now if both are on the element on the first load or if they are changed via the setter. The demo page should be working too.

Lemme know what you think or if there's anymore changes needed!

[muan]

Just one thing here, could you make sure it follows the current codestyle? ; and ". I should've added a ts-standard lint. Thanks!

[JamesIves]

👍🏻 done

[muan]

2. setter should update the content attribute

I see that this is still not the case?

I can do this but don't have time yet. Thanks for the work so far.
Once that's done I can merge it, or I can do a final check, add that, and merge it as well.

[JamesIves]

Just to clarify are you looking for the internal value of src and duration to be reflected back onto the element? For instance, if I were to set:

<open-stories src="./empty.json" duration="5"></open-stories>

It would reflect back onto the element after it performs all of the URL changes?

<open-stories src="https://example.com/empty.json" duration="5"></open-stories>

Thinking this through some more this morning, how do you feel about these refactors? this._src and this._duration are gone, and instead it's just calling getAttribute and setAttribute within the setters/getters.

  set src(path: string) {
    this.setAttribute("src", this.formatSrc(path));
  }

  get src(): string {
    return this.getAttribute("src") || "";
  }

  set duration(value: number) {
    this.setAttribute("duration", String(value));
  }

  get duration(): number {
    return Number(this.getAttribute("duration") || 5);
  }

  static get observedAttributes(): string[] {
    return ["src", "duration"];
  }

  attributeChangedCallback(
    attr: string,
    oldValue: string,
    newValue: string
  ): void {
    if (attr === "duration" && oldValue !== newValue) {
      this.duration = Number(newValue);
    }

    if (attr === "src" && oldValue !== newValue) {
      this.src = newValue;
    }
  }

This results in the following behavior when setAttribute is called on the element:

Screen.Recording.2023-09-13.at.9.48.29.AM.mov

[muan]

It would reflect back onto the element after it performs all of the URL changes?

No, the content attribute to should reflect what was set, but the getter should have the fully formed absolute URL. Using form.action again as a demo (sorry I should've included this in earlier examples):

$ form = document.createElement('form')
<form>​</form>​
$ form.action = 'x'
'x'
$ form
<form action=​"x">​</form>​
$ form.action
'https://github.com/dddddddddzzzz/open-stories-element/pull/x'

I do think observedAttributes and attributeChangedCallback are necessary, but we might be able to tackle that in another PR considering that it'll include some lifecycle resetting.

[muan]

Changes published in 0.0.25. Thank you!

Tracking remaining work in #15.