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

Can i create a theme-creativeshop grandchild? #73

Open
marcoveeneman opened this issue Apr 21, 2021 · 7 comments
Open

Can i create a theme-creativeshop grandchild? #73

marcoveeneman opened this issue Apr 21, 2021 · 7 comments

Comments

@marcoveeneman
Copy link

marcoveeneman commented Apr 21, 2021

Hi,

I'm currently doing some experiments with theme-creativeshop. I created a child theme which is working perfectly! I extended the child theme and would like to create another child based on the child, resulting in the following inheritance structure:

(parent)                (child)       (grandchild)
theme-creativeshop  <-  my-child  <-  my-child-child

I generated a new child using mtg according to the guide and changed the following line in grandchild/src/theme.xml:

<parent>Creativestyle/theme-creativeshop</parent>

to

<parent>Creativestyle/my-child</parent>

However, when building the grandchild i encounter the following error in yarn build:

yarn build
yarn run v1.22.5
$ gulp build --env production
[19:03:56] Using gulpfile ~/dev/magento/vendor/creativestyle/my-child-child/gulpfile.js
[19:03:56] Starting 'build'...
[19:03:56] Starting 'clean'...
[19:03:56] Finished 'clean' after 44 ms
[19:03:56] Starting 'collectViewXml'...
[19:03:56] Finished 'collectViewXml' after 22 ms
[19:03:56] Starting 'buildWebpack'...
[19:03:56] Starting 'copyHtml'...
[19:03:56] Starting 'copyScripts'...
[19:03:56] Starting 'copyImages'...
[19:03:56] Starting 'copyUnchanged'...
[19:03:56] 'buildWebpack' errored after 112 ms
[19:03:56] WebpackOptionsValidationError: Invalid configuration object. Webpack has been initialised using a configuration object that does not match the API schema.
 - configuration.entry should be an non-empty object.
   -> Multiple entry bundles are created. The key is the chunk name. The value can be a string or an array.
    at webpack (/data/web/dev/magento/vendor/creativestyle/my-child-child/node_modules/webpack/lib/webpack.js:31:9)
    at buildWebpack (/data/web/dev/magento/vendor/creativestyle/my-child-child/node_modules/@creativestyle/magesuite-frontend-builder/gulp/tasks/buildWebpack.js:15:22)
    at taskWrapper (/data/web/dev/magento/vendor/creativestyle/my-child-child/node_modules/undertaker/lib/set-task.js:13:15)
    at bound (domain.js:402:14)
    at runBound (domain.js:415:12)
    at asyncRunner (/data/web/dev/magento/vendor/creativestyle/my-child-child/node_modules/async-done/index.js:55:18)
    at process._tickCallback (internal/process/next_tick.js:61:11)
[19:03:56] 'build' errored after 191 ms
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Reverting the parent to theme-creativeshop the theme builds again (making it child instead of grandchild). I now start to wonder if this scenario is supported or not (yet?) hence i opened this ticket. Can i create a grandchild of theme-creativeshop? Am i doing something wrong?

@marcoveeneman
Copy link
Author

I managed to figure out what went wrong. Going to answer my own question here in case anyone will face the same issue.

It turns out the parentAliases.js file from the frontend-builder package is making assumptions about the name of the parent theme.

The parent theme is matched using the following regex: /<parent>[a-z]+\/[^\-]+\-([a-z]+)<\/parent>/i, in other words, it matches theme-creativeshop as well as my-child, but not my-child-child, it is assumed a theme will have only one dash in the name.

Then when a match has been found in parentAliases.js, the parentPath is resolved using path.resolve('../theme-${parentName}');, constructing the path to the parent theme. Here the assumption is made a theme is always starting with theme, which was not the case for me, i used my-child, resulting in a parentPath of ../theme-child instead of ../my-child.

parentAliases.js snippet
...
    const parentMatch = themeXML.match(
        /<parent>[a-z]+\/[^\-]+\-([a-z]+)<\/parent>/i
    );

    if (parentMatch) {
        const parentName = parentMatch[1];
        const parentPath = path.resolve(`../theme-${parentName}`);
...

Conclusion

A theme must be named so that it will match the following regex: /creativestyle\/theme-([a-z]+)/i or it will not work.

This will work:
✅ creativestyle/theme-customchild
✅ creativestyle/theme-customgrandchild

This will not work:
❌ anotherstyle/theme-creativeshop
❌ creativestyle/theme-custom-child
❌ creativestyle/custom-child

@marcoveeneman
Copy link
Author

I'm not sure if current behaviour is expected, feel free to close this issue if it is.

@krisdante
Copy link
Contributor

It would be nice to add support for
anothercompany/theme-whatever
This would make the biggest benefit.

When we create a child theme now for a customer we name it cust creativestyle/theme-customer. Theme to clearly show it is a theme. And then no dashes. This is by design.
But the fact that the namespace needs to be creativestyle is there just because we never had a need to use another one.

It would be nice if somebody adds this support.

@krisdante
Copy link
Contributor

I checked our projects, we we see that we have themes like
creativestyle/theme-customer-b2b
and that works.

I am not sure why -child is not working for you.

@marcoveeneman
Copy link
Author

Thanks for checking. The creativestyle/theme-customer-b2b theme itself will work fine. However, when creating a child of creativestyle/theme-customer-b2b, like creativestyle/theme-customer-b2b-child, it will not work because the parent it will search for is creativestyle/theme-customer. At least that is what i am seeing 🤔

@marcoveeneman
Copy link
Author

It would be nice to add support for anothercompany/theme-whatever This would make the biggest benefit.

When we create a child theme now for a customer we name it cust creativestyle/theme-customer. Theme to clearly show it is a theme. And then no dashes. This is by design. But the fact that the namespace needs to be creativestyle is there just because we never had a need to use another one.

It would be nice if somebody adds this support.

I added support for this in a PR in the frontend-builder repo. Would be great if it gets accepted 😄

@diwipl
Copy link
Contributor

diwipl commented Oct 7, 2021

Thank you for your contribution.

We will check your implementation internally and in case everything will be fine, we will merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants