Verified Commit ea8be0f9 authored by Dave Long's avatar Dave Long
Browse files

task: #3465228 Twig disallows dashes in variable names, so SDC should disallow... 

task: #3465228 Twig disallows dashes in variable names, so SDC should disallow it in prop names and slots

By: pdureau
By: vensires
By: lirak
By: kwiseman
By: penyaskito
By: wim leers
(cherry picked from commit 7dd6a4c9d48b1939b1e2bf018c6997a5638cda40)
parent a6c226b3

core/assets/schemas/v1/metadata-full.schema.json

+16 −2
Changes for core/assets/schemas/v1/metadata-full.schema.json: 16 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -3,7 +3,20 @@ 
  "$schema": "http://json-schema.org/draft-04/schema#",

  "$defs": {

    "propDefinition": {

      "$ref": "http://json-schema.org/draft-04/schema#",

      "$comment": "Prop names are exposed as Twig variables in the component template. Twig identifiers must start with a letter or underscore and cannot contain dashes, so prop names must match ^[a-zA-Z_][a-zA-Z0-9_]*$. Same pattern as slotDefinition.",

      "allOf": [

        { "$ref": "http://json-schema.org/draft-04/schema#" },

        {

          "properties": {

            "properties": {

              "additionalProperties": false,

              "patternProperties": {

                "^[a-zA-Z_][a-zA-Z0-9_]*$": {}

              }

            }

          }

        }

      ],

      "meta:enum": {

        "type": "object",

        "minItems": 1,
@@ -22,9 +35,10 @@ 
    },

    "slotDefinition": {

      "type": "object",

      "$comment": "Slot names are exposed as Twig block names in the component template. Twig identifiers must start with a letter or underscore and cannot contain dashes, so slot names must match ^[a-zA-Z_][a-zA-Z0-9_]*$. Same pattern as propDefinition",

      "additionalProperties": false,

      "patternProperties": {

        "^[a-zA-Z0-9_-]+$": {

        "^[a-zA-Z_][a-zA-Z0-9_]*$": {

          "type": "object",

          "properties": {

            "title": {

core/assets/schemas/v1/metadata.schema.json

+17 −2
Changes for core/assets/schemas/v1/metadata.schema.json: 17 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -3,7 +3,20 @@ 
  "$schema": "http://json-schema.org/draft-04/schema#",

  "$defs": {

    "propDefinition": {

      "$ref": "http://json-schema.org/draft-04/schema#",

      "$comment": "Prop names are exposed as Twig variables in the component template. Twig identifiers must start with a letter or underscore and cannot contain dashes, so prop names must match ^[a-zA-Z_][a-zA-Z0-9_]*$. Same pattern as slotDefinition.",

      "allOf": [

        { "$ref": "http://json-schema.org/draft-04/schema#" },

        {

          "properties": {

            "properties": {

              "additionalProperties": false,

              "patternProperties": {

                "^[a-zA-Z_][a-zA-Z0-9_]*$": {}

              }

            }

          }

        }

      ],

      "meta:enum": {

        "type": "object",

        "minItems": 1,
@@ -22,9 +35,10 @@ 
    },

    "slotDefinition": {

      "type": "object",

      "$comment": "Slot names are exposed as Twig block names in the component template. Twig identifiers must start with a letter or underscore and cannot contain dashes, so slot names must match ^[a-zA-Z_][a-zA-Z0-9_]*$. Same pattern as propDefinition",

      "additionalProperties": false,

      "patternProperties": {

        "^[a-zA-Z0-9_-]+$": {

        "^[a-zA-Z_][a-zA-Z0-9_]*$": {

          "type": "object",

          "properties": {

            "title": {
@@ -69,6 +83,7 @@ 
    },

    "variantDefinition": {

      "type": "object",

      "$comment": "Variant names may contain dashes because they are used as string values (CSS modifiers, equality checks) and are never converted into Twig variable or block names.",

      "additionalProperties": false,

      "patternProperties": {

        "^[a-zA-Z0-9_-]+$": {

core/lib/Drupal/Core/Theme/Component/ComponentValidator.php

+15 −0
Changes for core/lib/Drupal/Core/Theme/Component/ComponentValidator.php: 15 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -65,6 +65,21 @@ public function validateDefinition(array $definition, bool $enforce_schemas): bo 
      );

      throw new InvalidComponentException($message);

    }

    // Prop/slot names are exposed as Twig identifiers in the component

    // template, and those must start with a letter or underscore and cannot

    // contain dashes.

    // @see https://www.php.net/manual/en/language.variables.basics.php

    $invalid_prop_names = array_filter(

      $prop_names,

      static fn(string $name) => !preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*$/', $name),

    );

    if ($invalid_prop_names) {

      throw new InvalidComponentException(sprintf(

        'The component "%s" declares prop(s) with invalid name(s): [%s]. Prop names must match the pattern "^[a-zA-Z_][a-zA-Z0-9_]*$" so they can be used as Twig variables.',

        $definition['id'],

        implode(', ', $invalid_prop_names),

      ));

    }

    // If the validator isn't set, then the validation library is not installed.

    if (!$this->validator) {

      return TRUE;

core/tests/Drupal/Tests/Core/Theme/Component/ComponentValidatorTest.php

+44 −0
Changes for core/tests/Drupal/Tests/Core/Theme/Component/ComponentValidatorTest.php: 44 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -57,6 +57,18 @@ public static function dataProviderValidateDefinitionValid(): \Generator { 
    foreach ($data as $definition) {

      yield [$definition];

    }



    // Variant names may contain dashes because they are used as string values

    // (e.g. CSS modifiers) and are never converted to Twig variable or block

    // names, unlike props and slots.

    $cta_with_dashed_variant = static::loadComponentDefinitionFromFs('my-cta');

    $cta_with_dashed_variant['variants'] = [

      'my-variant' => [

        'title' => 'My variant',

        'description' => 'Variant names with dashes are valid.',

      ],

    ];

    yield 'variant name with dashes is valid' => [$cta_with_dashed_variant];

  }



  /**
@@ -140,6 +152,38 @@ public static function dataProviderValidateDefinitionInvalid(): \Generator { 
    ];

    yield 'invalid slot (name with spaces)' => [$cta_with_invalid_slot_name];



    $cta_with_dashed_slot_name = $valid_cta;

    $cta_with_dashed_slot_name['slots'] = [

      'invalid-slot' => [

        'title' => 'Invalid slot',

        'description' => 'Slot name cannot contain dashes because Twig disallows them in variable names.',

      ],

    ];

    yield 'invalid slot (name with dashes)' => [$cta_with_dashed_slot_name];



    $cta_with_dashed_prop_name = $valid_cta;

    $cta_with_dashed_prop_name['props']['properties']['my-prop'] = [

      'type' => 'string',

      'title' => 'My prop',

    ];

    yield 'invalid prop (name with dashes)' => [$cta_with_dashed_prop_name];



    $cta_with_leading_digit_prop_name = $valid_cta;

    $cta_with_leading_digit_prop_name['props']['properties']['1prop'] = [

      'type' => 'string',

      'title' => '1prop',

    ];

    yield 'invalid prop (name starts with digit)' => [$cta_with_leading_digit_prop_name];



    $cta_with_leading_digit_slot_name = $valid_cta;

    $cta_with_leading_digit_slot_name['slots'] = [

      '1slot' => [

        'title' => 'Invalid slot',

        'description' => 'Slot name cannot start with a digit because Twig identifiers must start with a letter or underscore.',

      ],

    ];

    yield 'invalid slot (name starts with digit)' => [$cta_with_leading_digit_slot_name];



    $cta_with_invalid_variant_title_type = $valid_cta;

    $cta_with_invalid_variant_title_type['variants'] = [

      'valid_variant' => [