Issue #2705037 by jhedstrom, Dinesh18, dawehner, Wim Leers, larowlan: Asset library DX: Non-helpful fatal error if CSS isn't nested under an existing category
							parent
							
								
									9eec58d248
								
							
						
					
					
						commit
						04573c0632
					
				| 
						 | 
				
			
			@ -129,13 +129,17 @@ class LibraryDiscoveryParser {
 | 
			
		|||
        //   properly resolve dependencies for all (css) libraries per category,
 | 
			
		||||
        //   and only once prior to rendering out an HTML page.
 | 
			
		||||
        if ($type == 'css' && !empty($library[$type])) {
 | 
			
		||||
          assert('\Drupal\Core\Asset\LibraryDiscoveryParser::validateCssLibrary($library[$type]) < 2', 'CSS files should be specified as key/value pairs, where the values are configuration options. See https://www.drupal.org/node/2274843.');
 | 
			
		||||
          assert('\Drupal\Core\Asset\LibraryDiscoveryParser::validateCssLibrary($library[$type]) === 0', 'CSS must be nested under a category. See https://www.drupal.org/node/2274843.');
 | 
			
		||||
          foreach ($library[$type] as $category => $files) {
 | 
			
		||||
            $category_weight = 'CSS_' . strtoupper($category);
 | 
			
		||||
            assert('defined($category_weight)', 'Invalid CSS category: ' . $category . '. See https://www.drupal.org/node/2274843.');
 | 
			
		||||
            foreach ($files as $source => $options) {
 | 
			
		||||
              if (!isset($options['weight'])) {
 | 
			
		||||
                $options['weight'] = 0;
 | 
			
		||||
              }
 | 
			
		||||
              // Apply the corresponding weight defined by CSS_* constants.
 | 
			
		||||
              $options['weight'] += constant('CSS_' . strtoupper($category));
 | 
			
		||||
              $options['weight'] += constant($category_weight);
 | 
			
		||||
              $library[$type][$source] = $options;
 | 
			
		||||
            }
 | 
			
		||||
            unset($library[$type][$category]);
 | 
			
		||||
| 
						 | 
				
			
			@ -460,4 +464,34 @@ class LibraryDiscoveryParser {
 | 
			
		|||
    return $overriding_asset;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * Validates CSS library structure.
 | 
			
		||||
   *
 | 
			
		||||
   * @param array $library
 | 
			
		||||
   *   The library definition array.
 | 
			
		||||
   *
 | 
			
		||||
   * @return int
 | 
			
		||||
   *   Returns based on validity:
 | 
			
		||||
   *     - 0 if the library definition is valid
 | 
			
		||||
   *     - 1 if the library definition has improper nesting
 | 
			
		||||
   *     - 2 if the library definition specifies files as an array
 | 
			
		||||
   */
 | 
			
		||||
  public static function validateCssLibrary($library) {
 | 
			
		||||
    $categories = [];
 | 
			
		||||
    // Verify options first and return early if invalid.
 | 
			
		||||
    foreach ($library as $category => $files) {
 | 
			
		||||
      if (!is_array($files)) {
 | 
			
		||||
        return 2;
 | 
			
		||||
      }
 | 
			
		||||
      $categories[] = $category;
 | 
			
		||||
      foreach ($files as $source => $options) {
 | 
			
		||||
        if (!is_array($options)) {
 | 
			
		||||
          return 1;
 | 
			
		||||
        }
 | 
			
		||||
      }
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    return 0;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -502,6 +502,47 @@ class LibraryDiscoveryParserTest extends UnitTestCase {
 | 
			
		|||
    $this->assertEquals($library['license'], $expected_license);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * Verifies assertions catch invalid CSS declarations.
 | 
			
		||||
   *
 | 
			
		||||
   * @dataProvider providerTestCssAssert
 | 
			
		||||
   */
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * Verify an assertion fails if CSS declarations have non-existent categories.
 | 
			
		||||
   *
 | 
			
		||||
   * @param string $extension
 | 
			
		||||
   *   The css extension to build.
 | 
			
		||||
   * @param string $exception_message
 | 
			
		||||
   *   The expected exception message.
 | 
			
		||||
   *
 | 
			
		||||
   * @dataProvider providerTestCssAssert
 | 
			
		||||
   */
 | 
			
		||||
  public function testCssAssert($extension, $exception_message) {
 | 
			
		||||
    $this->moduleHandler->expects($this->atLeastOnce())
 | 
			
		||||
      ->method('moduleExists')
 | 
			
		||||
      ->with($extension)
 | 
			
		||||
      ->will($this->returnValue(TRUE));
 | 
			
		||||
 | 
			
		||||
    $path = __DIR__ . '/library_test_files';
 | 
			
		||||
    $path = substr($path, strlen($this->root) + 1);
 | 
			
		||||
    $this->libraryDiscoveryParser->setPaths('module', $extension, $path);
 | 
			
		||||
 | 
			
		||||
    $this->setExpectedException(\AssertionError::class, $exception_message);
 | 
			
		||||
    $this->libraryDiscoveryParser->buildByExtension($extension);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * Data provider for testing bad CSS declarations.
 | 
			
		||||
   */
 | 
			
		||||
  public function providerTestCssAssert() {
 | 
			
		||||
    return [
 | 
			
		||||
      'css_bad_category' => ['css_bad_category', 'See https://www.drupal.org/node/2274843.'],
 | 
			
		||||
      'Improper CSS nesting' => ['css_bad_nesting', 'CSS must be nested under a category. See https://www.drupal.org/node/2274843.'],
 | 
			
		||||
      'Improper CSS nesting array' => ['css_bad_nesting_array', 'CSS files should be specified as key/value pairs, where the values are configuration options. See https://www.drupal.org/node/2274843.'],
 | 
			
		||||
    ];
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
/**
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -0,0 +1,5 @@
 | 
			
		|||
bad_category:
 | 
			
		||||
  css:
 | 
			
		||||
    # Non-existent category.
 | 
			
		||||
    bad_category:
 | 
			
		||||
      css/styles.css: { minified: true }
 | 
			
		||||
| 
						 | 
				
			
			@ -0,0 +1,4 @@
 | 
			
		|||
bad_nesting:
 | 
			
		||||
  css:
 | 
			
		||||
    # No nesting here will break.
 | 
			
		||||
    css/styles.css: { minified: true }
 | 
			
		||||
| 
						 | 
				
			
			@ -0,0 +1,4 @@
 | 
			
		|||
bad_nesting_array:
 | 
			
		||||
  css:
 | 
			
		||||
    # Specified as an array will break.
 | 
			
		||||
    - css/styles.css
 | 
			
		||||
| 
						 | 
				
			
			@ -1,6 +1,7 @@
 | 
			
		|||
example:
 | 
			
		||||
  css:
 | 
			
		||||
    css/example.js: {}
 | 
			
		||||
    theme:
 | 
			
		||||
      css/example.css: {}
 | 
			
		||||
  dependencies:
 | 
			
		||||
    - external/example_external
 | 
			
		||||
    - example_module/example
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in New Issue