Removed .env configuration #629
Conversation
|
I know that some variables will remain empty, but the build seems ok like this. Removing .env dependency will help us to ship hummingbird in prestashop core. |
There was a problem hiding this comment.
The env configuration was added for a reason. 🤔 Ping @Oksydan @NeOMakinG
|
I know that there were some reasons ... but we can't find them. |
|
PR that introduced this, with discussion - #67 |
|
OK so it seems that you warned about this issue... |
|
@nicosomb .env file is being used by webpack-dev-server mostly to improve DX. Why would you like to get ride of this like this 🤔. |
|
@Oksydan I understand, but can you tell us how can we ship it in PrestaShop core please ? We want to add Hummingbird as a dependency in composer.json (see my PR here PrestaShop/PrestaShop#36198). So we have to know how to fill the .env file during the assets build process. |
|
Hmmmmm we could pass arguments in the compilation process. If the process is building a theme, we could simply not check if the const {
PORT: port = null,
PUBLIC_PATH: publicPath = null,
SERVER_ADDRESS: serverAddress = null,
SITE_URL: siteURL = null,
} = process.env;THB we do not need these values in the compilation process. It is only required for the development process. |
|
@Oksydan thank you for the details. I will improve my PR. |
nicosomb
force-pushed
the
remove-env-config
branch
from
76f0070 to
35ac6b6
Compare
| @@ -176,7 +176,7 @@ | |||
| {/block} | |||
| </div> | |||
|
|
|||
| {if $product.add_to_cart_url && $product.product_type !== 'combinations'} | |||
| {if $product.add_to_cart_url} | |||
There was a problem hiding this comment.
This is related to this PR: #609
There was a problem hiding this comment.
I know that. But it seems that it breaks UI tests.
There was a problem hiding this comment.
I think that's an issue about UI tests then.
If we merge your diff, we reintroduce the "Add to cart" button for combinations products, and his buggy behaviour in that particular case 🤔
But anyway, I don't block your PR, just I prefer to warn you about this! 😉
There was a problem hiding this comment.
I think the change is legit, add to cart url should be handled on product lazy array settings. For combination, it's availability depends on prestashop settings (enable add to cart button in listings when product has combinations on/off).
nicosomb
force-pushed
the
remove-env-config
branch
from
35ac6b6 to
1f94b21
Compare
nicosomb
force-pushed
the
remove-env-config
branch
from
1f94b21 to
103cdde
Compare
|
@Hlavtox we have to go forward, could you please remove your request approval? |
|
Hello @nicosomb , What are the steps to reproduce ? |
There was a problem hiding this comment.
Hello @nicosomb ,
Hummingbird is well installed.
Made an order with all types of products, order process is working as expected.
LGTM ✅
Thanks!
This PR allows to fix the issue I have here PrestaShop/PrestaShop#36198 (comment) to embed HB in PrestaShop core.