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

Pattern Lab only loads the pattern engines from the config #1256

Merged
merged 26 commits into from
Dec 30, 2022

Conversation

ringods
Copy link
Contributor

@ringods ringods commented Sep 1, 2020

Closes #1251

Summary of changes:

  • Load the engines which are configured in the config first
  • Fallback to the old way of scanning node_modules if no engines are configured
  • Documentation added on the engines property in the Pattern Lab config file. URLs in the source code should point to this property once it is published during the release.
  • Proper deprecation messages should be in place.

@coveralls
Copy link

coveralls commented Sep 1, 2020

Coverage Status

Coverage decreased (-1.8%) to 75.0% when pulling 5d376a5 on feature/resolver-pattern-engines into 1d28ab0 on dev.

@JosefBredereck
Copy link
Contributor

JosefBredereck commented Sep 2, 2020

I can not approve that PR at the moment. The preview is not building.

Pattern Engine undefined: good to go
Found plugin: @pattern-lab/plugin-tab
Attempting to load and initialize plugin.
where is the engine?
Error rendering pattern lab header
Omitting atoms-colors from styleguide patterns because it has an underscore prefix.
where is the engine?
error during header render()
where is the engine?
error during footer render()
where is the engine?
Error building buildFooterHTML

@JosefBredereck
Copy link
Contributor

When I add engine mustache, it is working. (We need to get rid of that engine to be required everywhere but it's part of the UI-kit) But it still messages:
Pattern Engine undefined: good to go
Pattern Engine undefined: good to go

[...]
    "mustache": {
      "package": "@pattern-lab/engine-mustache",
      "fileExtensions": [
        "mustache"
      ]
    }
[...]

@stale
Copy link

stale bot commented Dec 19, 2020

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!

@JosefBredereck JosefBredereck removed the request for review from raphaelokon January 12, 2021 08:59
@mfranzke mfranzke changed the title PatternLab only loads the pattern engines from the config Pattern Lab only loads the pattern engines from the config Dec 24, 2022
@mfranzke mfranzke marked this pull request as draft December 26, 2022 12:56
@mfranzke
Copy link
Contributor

mfranzke commented Dec 26, 2022

@ringods first of all thanks a lot for your work that you did so far regarding this topic. I tried to follow up on the feedback by @JosefBredereck and checked in some minor changes of how I would think about solving this topic. I hope that was in your interest.

I've integrated some workarounds, that we should be able to remove with #1455; as even also mentioned by @JosefBredereck we should get rid of mustache in favour of its successor handlebars anyhow.

@mfranzke mfranzke marked this pull request as ready for review December 28, 2022 07:03
packages/docs/src/docs/advanced-config-options.md Outdated Show resolved Hide resolved
if (enginesInConfig) {
// Quick fix until we've removed @pattern-lab/engine-mustache, starting with https://github.com/pattern-lab/patternlab-node/issues/1239
// @TODO: Remove after removing @pattern-lab/engine-mustache dependency
enginesInConfig.mustache = enginesInConfig.mustache || {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting approach. Do we need to change this to handlebars after you changed the package with #1456?

Copy link
Contributor

@mfranzke mfranzke Dec 30, 2022

Choose a reason for hiding this comment

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

It's mainly a mitigation, because we still need the mustache engine as we reference mustache templates all over the place, and without it we would get errors like that the header isn't available. By #1455 we should be able to remove this code part again.

@JosefBredereck JosefBredereck merged commit 470d37c into dev Dec 30, 2022
@JosefBredereck JosefBredereck deleted the feature/resolver-pattern-engines branch December 30, 2022 16:54
mfranzke added a commit that referenced this pull request Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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