Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[emulsify-ds/compound/issues/57] Dependency Improvements & Refactoring & Renaming #269

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .storybook/preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Twig from 'twig';
import { setupTwig } from './setupTwig';

// GLOBAL CSS
import '../components/style.scss';
import '../components/global.scss';

// If in a Drupal project, it's recommended to import a symlinked version of drupal.js.
import './_drupal.js';
Expand Down
4 changes: 4 additions & 0 deletions .storybook/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ module.exports = async ({ config }) => {
sassOptions: {
importer: globImporter(),
},
additionalData: `@import "${path.resolve(
__dirname,
'../components/_import.scss',
)}";`,
},
},
],
Expand Down
File renamed without changes.
4 changes: 0 additions & 4 deletions emulsify.info.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ libraries:
# component:
# css/chosen-drupal.css: false

# CKEditor stylesheet loads in wysiwyg to give content editors a better experience
ckeditor_stylesheets:
- dist/css/style.css

regions:
header: Header
content: Content # the content region is required
Expand Down
14 changes: 1 addition & 13 deletions emulsify.libraries.yml
Original file line number Diff line number Diff line change
@@ -1,23 +1,11 @@
global:
css:
theme:
dist/css/style.css: {}

main-menu:
js:
dist/js/02-molecules/menus/main-menu/main-menu.js: {}
dependencies:
- core/drupal
dist/css/global.css: {}

# IE 11 support for SVG use - disable if not needed.
# See also components/01-atoms/images/icons/_icon.twig to remove attach_library.
sprite:
js:
components/01-atoms/images/icons/svgxuse.min.js:
{ attributes: { defer: true } }

tabs:
js:
dist/js/02-molecules/tabs/tabs.js: {}
dependencies:
- core/drupal
102 changes: 102 additions & 0 deletions emulsify.theme
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,105 @@
* @file
* Functions to support theming.
*/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might make more sense to go in the Emulsify Twig module rather than in each theme. If it's going to be the recommended way to write libraries, I'd hate for someone to not know what this is, and delete it from their theme, and then ask us why their libraries don't work... If it's in the module (which is a Drupal dependency) they can't delete it, and keeps "required" code out of their custom codebase.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To do that, we also need to change the code a little bit. (Need to get the active theme and dir)

$theme_name = basename(__FILE__, '.theme'); // compony

Or we can just wait for this feature to become included in the core https://www.drupal.org/project/drupal/issues/3092496

We can propagate the idea of If you don't know and if all works good, DON'T touch anything! 😄

/**
* Implements hook_library_info_alter().
* @link https://www.drupal.org/project/drupal/issues/3092496
*/
function emulsify_library_info_alter(&$libraries, $extension) {
// Get the name of the theme where this function is being called
$theme_name = basename(__FILE__, '.theme'); // compony

// Get the path of the theme where this function is being called
$theme_path = drupal_get_path('theme', $theme_name); // themes/custom/compony

// Alter only the library definitions of the current theme.
if ($extension == $theme_name) {
$directory_iterator = new RecursiveDirectoryIterator($theme_path . '/components/');

// Iterate over all the files found in /themes/custom/compony/components/
foreach (new RecursiveIteratorIterator($directory_iterator) as $file) {
// Filter out all the files that have the exact name: "libraries.yml"
if ($file->getFilename() == 'libraries.yml') {
try {
// Let's assume we found a libraries.yml file on /themes/custom/compony/components/status-messages/libraries.yml
$componentPathFromRoot = substr($file->getPathName(), 0, -13); // /themes/custom/compony/components/status-messages/
$componentPathFromTheme = str_replace($theme_path . '/', '', $componentPathFromRoot); // /components/status-messages/

// Decode the libraries.yml
$new_libraries = Drupal\Component\Serialization\Yaml::decode(file_get_contents($file->getRealPath()));

// Each libraries.yml could have multiple library-definitions
foreach ($new_libraries as $key => $new_library) {
// Check if the key of this library "status-messages" in our example isn't already defined somewhere else
if(isset($libraries[$key])) {
// If the library is defined somewhere else already,
// throw a warning that we have multiple definitions of
// the same library within the same theme.
\Drupal::messenger()
->addWarning(t('The library @key from the theme @themename has multiple definitions.', [
'@key' => $key,
'@themename' => $theme_name,
]));
} else {
// If the library key hasn't been defined yet, and the library contains CSS definitions
if (isset($new_library['css'])) {
// Go over each CSS group definition, multiple groups are possible, so we need to loop over them
foreach($new_library['css'] as $group_key => $css_grouped) {
// Inside each group definition, there can be multiple CSS-files, so again we need to loop over those
foreach($css_grouped as $file_key => $css_file) {
// If the path to the file is absolutely defined, for example:
// components/status-messages/dist/status-messages.css,
// then it will work out of the box.
if(substr($file_key, 0, 5) == 'dist/') {
// no need to do anything
} else {
// If a type is defined
if (isset($css_file['type'])) {
// If the type of the css file is external
if ($css_file['type'] == 'external') {
// break out of the foreach loop of this CSS-file.
continue;
}
}

// We only arrive here if the path doesn't start with 'components/',
// which would indicate the path is relative.
// We prefix the path to the css file of the relative definition with $componentPathFromTheme,
// so internally it will be absolute positioned starting from the theme it is found in.
$new_library['css'][$group_key][$componentPathFromTheme . $file_key] = $css_file;
unset($new_library['css'][$group_key][$file_key]);
}
}
}
}

// Do the same for the JS as we did for CSS
if (isset($new_library['js'])) {
foreach($new_library['js'] as $file_key => $js_file) {
if(substr($file_key, 0, 5) == 'dist/') {
} else {
if (isset($js_file['type'])) {
if ($js_file['type'] == 'external') {
continue;
}
}

// Path is relatively defined
$new_library['js'][$componentPathFromTheme . $file_key] = $js_file;
unset($new_library['js'][$file_key]);
}
}
}
// Set the libraries variable to now have the altered library.
$libraries[$key] = $new_library;
}
}
} catch (InvalidDataTypeException $e) {
// Throw a helpful exception to provide context.
throw new InvalidLibraryFileException(sprintf('Invalid library definition in %s: %s', $file->getRealPath(), $e->getMessage()), 0, $e);
}
}
}
};
}
1 change: 0 additions & 1 deletion webpack/css.js

This file was deleted.

1 change: 1 addition & 0 deletions webpack/global.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import '../components/global.scss';
12 changes: 12 additions & 0 deletions webpack/loaders.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const path = require('path');
const MiniCssExtractPlugin = require('mini-css-extract-plugin');
const globImporter = require('node-sass-glob-importer');

Expand Down Expand Up @@ -35,6 +36,10 @@ const CSSLoader = {
loader: 'sass-loader',
options: {
sourceMap: true,
additionalData: `@import "${path.resolve(
__dirname,
'../components/_import.scss',
)}";`,
sassOptions: {
importer: globImporter(),
outputStyle: 'compressed',
Expand All @@ -53,9 +58,16 @@ const SVGSpriteLoader = {
},
};

const FontLoader = {
test: /.(woff|woff2|ttf|eot|otf|svg)$/,
loader: 'file-loader',
include: [/fonts/],
};

module.exports = {
JSLoader,
CSSLoader,
SVGSpriteLoader,
ImageLoader,
FontLoader,
};
16 changes: 16 additions & 0 deletions webpack/plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ const _ImageminPlugin = require('imagemin-webpack-plugin').default;
const _SpriteLoaderPlugin = require('svg-sprite-loader/plugin');
const glob = require('glob');

const _StyleLintPlugin = require('stylelint-webpack-plugin');
const _ESLintPlugin = require('eslint-webpack-plugin');

const imagePath = path.resolve(__dirname, '../images');

const MiniCssExtractPlugin = new _MiniCssExtractPlugin({
Expand All @@ -29,6 +32,17 @@ const SpriteLoaderPlugin = new _SpriteLoaderPlugin({

const ProgressPlugin = new webpack.ProgressPlugin();

const StyleLintPlugin = new _StyleLintPlugin({
configFile: path.resolve(__dirname, '../', '.stylelintrc'),
context: path.resolve(__dirname, '../', 'components'),
files: '**/*.scss',
});

const ESLintPlugin = new _ESLintPlugin({
context: path.resolve(__dirname, '../', 'components'),
extensions: ['js'],
});

module.exports = {
ProgressPlugin,
MiniCssExtractPlugin,
Expand All @@ -46,4 +60,6 @@ module.exports = {
'!*.{png,jpg,gif,svg}',
],
}),
StyleLintPlugin,
ESLintPlugin,
};
40 changes: 16 additions & 24 deletions webpack/webpack.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,40 +7,23 @@ const webpackDir = path.resolve(__dirname);
const rootDir = path.resolve(__dirname, '..');
const distDir = path.resolve(rootDir, 'dist');

// Glob pattern for scss files that ignore file names prefixed with underscore.
const scssPattern = path.resolve(rootDir, 'components/**/!(_*).scss');
// Glob pattern for JS files.
const jsPattern = path.resolve(
rootDir,
'components/**/!(*.stories|*.component|*.min|*.test).js',
);

// Prepare list of scss and js file for "entry".
function getEntries(scssPattern, jsPattern) {
function getEntries(pattern, patternCss) {
const entries = {};

// SCSS entries
glob.sync(scssPattern).forEach((file) => {
glob.sync(pattern).forEach((file) => {
const filePath = file.split('components/')[1];
const newfilePath = `css/${filePath.replace('.scss', '')}`;
const newfilePath = `js/${filePath.replace('.js', '')}`;
entries[newfilePath] = file;
});

// JS entries
glob.sync(jsPattern).forEach((file) => {
glob.sync(patternCss).forEach((file) => {
const filePath = file.split('components/')[1];
const newfilePath = `js/${filePath.replace('.js', '')}`;
const newfilePath = `css/${filePath.replace('.component.scss', '')}`;
entries[newfilePath] = file;
});

entries.svgSprite = path.resolve(webpackDir, 'svgSprite.js');

// CSS Files.
glob.sync(`${webpackDir}/css/*js`).forEach((file) => {
const baseFileName = path.basename(file);
const newfilePath = `css/${baseFileName.replace('.js', '')}`;
entries[newfilePath] = file;
});
entries.global = path.resolve(webpackDir, 'global.js');

return entries;
}
Expand All @@ -49,13 +32,20 @@ module.exports = {
stats: {
errorDetails: true,
},
entry: getEntries(scssPattern, jsPattern),
entry: getEntries(
path.resolve(
rootDir,
'components/**/!(*.stories|*.component|*.min|*.mixin|*.test).js',
),
path.resolve(rootDir, 'components/**/*.component.scss'),
),
module: {
rules: [
loaders.CSSLoader,
loaders.SVGSpriteLoader,
loaders.ImageLoader,
loaders.JSLoader,
loaders.FontLoader,
],
},
plugins: [
Expand All @@ -64,6 +54,8 @@ module.exports = {
plugins.SpriteLoaderPlugin,
plugins.ProgressPlugin,
plugins.CleanWebpackPlugin,
plugins.StyleLintPlugin,
plugins.ESLintPlugin,
],
output: {
path: distDir,
Expand Down