Skip to content

fix(component): bug when extending legacy notation#570

Closed
redgeoff wants to merge 3 commits intomainfrom
condensed-extended-legacy-notation
Closed

fix(component): bug when extending legacy notation#570
redgeoff wants to merge 3 commits intomainfrom
condensed-extended-legacy-notation

Conversation

@redgeoff
Copy link
Copy Markdown
Owner

@redgeoff redgeoff commented Jan 16, 2022

The new condensed notation doesn't work when you extend a component with the legacy notation. This issue was detected by redgeoff/mson-react#316

@redgeoff
Copy link
Copy Markdown
Owner Author

redgeoff commented Jan 16, 2022

TODO: need to ponder this more, but the solution may be to refactor out all legacy notation so that the className can be overwritten when using the condensed notation. As you can see using the condensed notation when extending a component using the condensed notation works.

@redgeoff
Copy link
Copy Markdown
Owner Author

redgeoff commented Jan 17, 2022

As suspected, the condensed notation cannot be mixed with the legacy notation as the order of instantiation of className differs greatly. Consider the following example:

let n = 0;

const prefixString = (prefix) => {
  console.log(prefix, ++n);
  return `${prefix}${n}`;
}

class Component {
  className = 'Component';

  constructor(className) {
    this.className = className;
  }
}

class DerivedComponent extends Component {
  className = prefixString('DerivedComponent'); 
}

class DerivedDerivedComponent extends DerivedComponent {
  constructor() {
    super(prefixString('DerivedDerivedComponent'))
  }
}

console.log((new DerivedDerivedComponent()).className)

In this example console.log((new DerivedDerivedComponent()).className) outputs DerivedComponent2. Instead, we'd be looking for it to start with DerivedDerivedComponent.

If you look closer at the output of the console.log(prefix, ++n) statements, you'd see:

DerivedDerivedComponent 1
DerivedComponent 2

this is because:

constructor() {
  super(prefixString('DerivedDerivedComponent'))
}

is executed before:

className = prefixString('DerivedComponent');

which means that className is set to DerivedComponent and not DerivedDerivedComponent.

The key to achieving our desired behavior is to only set a className member variable it in the constructor:

class Component {
  constructor(className) {
    this.className = className ? className : 'Component';
  }
}

class DerivedComponent extends Component {
  constructor(className) {
    super(className ? className : 'DerivedComponent')
  }  
}

class DerivedDerivedComponent extends DerivedComponent {
  constructor() {
    super('DerivedDerivedComponent')
  }
}

With this, console.log((new DerivedDerivedComponent()).className) then outputs DerivedDerivedComponent

@redgeoff
Copy link
Copy Markdown
Owner Author

For now, opting not to support a className property. Instead, it has been documented as a possible future in enhancement by #574

@redgeoff redgeoff closed this Jan 18, 2022
@redgeoff redgeoff deleted the condensed-extended-legacy-notation branch January 18, 2022 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant