Request for input: make pattern engine loading explicit via the PL config

[ringods]

This issue is to discuss the changes in loading pattern engines. It requires input from the core team: @bmuenzenmeyer @bradfrost @sghoweri @JosefBredereck and whoever I am forgetting.

In #1225 & #1246, I changed the way how resources from UIKits are loaded: via the NodeJS resolver (require.resolve). For this to work, it now requires the property package, with a fallback at the moment based on name but which will be removed in the future. This makes it the loading explicit: first add the UIKit as a package dependency, then resolve the resources and copy them. Using the resolver also makes this feature working independent of the package manager.

The next piece of code loading that needs to go via the resolver is the loading of pattern engines. The current code is again scanning the node_modules folder in 2 different locations manually:

patternlab-node/packages/core/src/lib/pattern_engines.js

Lines 12 to 21 in 931b390

	const enginesDirectories = [
	{
	displayName: 'the core',
	path: path.resolve(__dirname, '..', '..', 'node_modules'),
	},
	{
	displayName: 'the edition or test directory',
	path: path.join(process.cwd(), 'node_modules'),
	},
	];

It returns every package which matches engine- in the package name:

patternlab-node/packages/core/src/lib/pattern_engines.js

Lines 23 to 37 in 931b390

	/**
	* Given a path: return the engine name if the path points to a valid engine
	* module directory, or false if it doesn't.
	* @param filePath
	* @returns Engine name if exists or FALSE
	*/
	function isEngineModule(filePath) {
	const baseName = path.basename(filePath);
	const engineMatch = baseName.match(engineMatcher);
	if (engineMatch) {
	return engineMatch[1];
	}
	return false;
	}

Like UIKits, this is not explicit and can lead to incorrect packages being found. If we want to make it explicit, the PatternLab configuration needs to be extended with an explicit mention of the pattern engines which must be loaded. But how should this look?

• Should the config only specify a single pattern engine or should it be a list?
• For each of the pattern engines, is more config needed besides the package name?

Any further suggestions?

[JosefBredereck]

I'm completely on your side on this topic. I would recommend defining by a package which engine should be loaded for engines, kits, plugins, and other stuff that we ever plan in the future. Only relying on the name could lead to unintentional errors, because of deep dependencies in other public packages.

Adding also my question from this ticket: #872 (comment) because it also references to the pluing loading by name and not package.

[bmuenzenmeyer]

I agree, everything should be explicit from the config!

…

On Sat, Aug 29, 2020, 3:12 PM Josef Bredreck ***@***.***> wrote: I'm completely on your side on this topic. I would recommend to define by a package which engine should be loaded for engines, kits, plugins, and other stuff that we ever plan in the future. Only relying on the name could lead to unintentional errors, because of deep dependencies in other public packages. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1251 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACI3Q5RIJOSKAKOFWYV7MTSDFOKJANCNFSM4QON6FFQ> .

[stale]

It's hard to keep track of everything. This issue has been automatically marked as stale because it has not had recent activity, neither from the team nor the community. It will be closed if no further activity occurs. Please consider adding additional info, volunteering to contribute a fix for this issue, or making a further case that this is important to you, the team, and the project as a whole. Thanks!

[stale]

It's hard to keep track of everything. This issue has been automatically marked as stale because it has not had recent activity, neither from the team nor the community. It will be closed if no further activity occurs. Please consider adding additional info, volunteering to contribute a fix for this issue, or making a further case that this is important to you, the team, and the project as a whole. Thanks!