One of the more difficult pieces of the AST to work with is properties. Why? Because computed-name properties and statically-named properties are a single AST node.

This is a huge footgun that most people don't think about when writing static analysis code or transforms on top of the AST. I see a lot of code which looks like this:

function analyseProperty(prop: Property) {
  if (property.key.type === 'Identifier') {
    analyseStaticPropertyName(property.key);
  }
}

This code looks correct at a quick glance, until you realise that if prop.computed === true, then an Identifier has a very different meaning:

const prop = 1;
const x = {
  prop: 2,   // declares a property named "prop" with the value 2
  [prop]: 2, // declares a property named "1" with the value 2
};

Often times these bugs lie dormant in code as computed properties aren't super-regularly used (in my experience), but the bug can cause some really frustrating linting experiences - or in the case of code transforms it can lead to broken code in production.

There's obviously no truly backwards-compatible way of fixing this. I have two ideas for how the AST might be updated to fix this.

Create a new node for computed-name properties

Currently my best idea is to create a new node like:

interface PropertyBase { /* ... */ }

interface Property extends PropertyBase {
  type: 'Property';
  key: Identifier | StringLiteral;
}
interface ComputedProperty extends PropertyBase {
  type: 'ComputedProperty';
  computedKey: Expression;
}

The reason you'd probably want a new key entirely is to stop people just updating to this pattern and falling into exactly the same state that existed before:

function analyseProperty(prop: Property | ComputedProperty) {
  if (property.key.type === 'Identifier') {
    analyseStaticPropertyName(property.key);
  }
}

PROs:

• Old tools that explicitly guard their object property iterations against SpreadElements will continue to work correctly if the guard was prop.type === 'Property'.

CONs:

• The variation in the key naming is kinda ugly.
• Tools that assume "if not SpreadElement, is instead a Property" will crash because .key is now potentially undefined.

Create a new node just for the property key

Another idea would be to rework the .key property:

interface StaticPropertyKey {
  type: 'StaticPropertyKey';
  key: Identifier | StringLiteral;
}

interface ComputedPropertyKey {
  type: 'ComputedPropertyKey';
  expression: Expression;
}

interface Property {
  // ...
  key: StaticPropertyKey | ComputedPropertyKey;
  // remove the computed property
}

Again - the two types have different property names to prevent passthrough chaining like:

function analyseProperty(prop: Property) {
  if (property.key.key.type === 'Identifier') {
    analyseStaticPropertyName(property.key);
  }
}

PROs:

• Cleaner approach as it nicely encapsulates the computed AST variation within the key, rather than it existing on the property.
• Will not explicitly crash any well-written tooling because checks like prop.key.type === 'Identifier' will now break.

CONs:

• Completely backwards incompatible because old tooling will no longer be able to analyse property keys.

I'd love to hear some thoughts about this.
Is this too much of a breaking change? Is this just an AST state that we need to grit our teeth and bear? Or is there something we could do about this?