search

Oksydan / falcon

Prestashop starter theme that provides great development experience.
GNU General Public License v3.0
248 stars 60 forks source link

🐛 [BUG] - Reproducible (release) build #236

Open cruftex opened 1 year ago

cruftex commented 1 year ago

Description

The build of the released theme should be documented and reproducible.

I currently cannot reproduce the theme build. Some portions of JS code seem to be missing in the final product. When doing the build outside the Prestashop installation, the build completes without error, but misses just a bit of JS code that leads to strange behavior once installed.

When comparing the JS code, I can see different function name replacements, also for the same code portions. I think there are some webpack settings for reproducible builds.

I think it would be good to either stick to yarn or npm for the instructions and the lock file. Experienced users should be able to use the other one, anyhow.

Maybe it would be good to fail the build when needed modules are missing. At least it would have saved a day of searching :)

Node.js version

v18.12.1

php version

n/a

OS and it's version

Ubuntu 22.04

Browsers

Chrome

Required module/theme

falcon

Reproduction steps

Ubuntu 22.04 and freshly created user with empty home directory
nvm installation, according to https://github.com/nvm-sh/nvm#install--update-script

`
git clone https://github.com/Oksydan/falcon.git
cd falcon
git checkout v3.1.1
cd _dev
nvm install
npm install --global yarn
yarn install
yarn build-ci

assets/js/theme.js is 77958 bytes on my build with the above commands assets/js/theme.js is 92116 bytes in falcon 3.1.1 release

Alternatively, building with npm I do instead of the last two steps:

npm install
npm run build-ci

The created theme.js file has the same size as above.

When I copy the theme to Prestashop and then do the build within this environment the JS file becomes bigger: 82169 bytes. The built theme seems to work properly, however, there is still 10k bytes difference to the "official" release which should be investigated.



### Logs

_No response_
Oksydan commented 1 year ago

Hi @cruftex,

Thank you for creating an issue. Releases sizes changes are comming from Workspaces Aware Webpacka feature https://github.com/Oksydan/falcon#workspace-aware-webpack. Theme is looking at modules directory and looking for modules with that specific structure. If it finds file, webpack will compile it and append to entry point.

I am using Makefile to release theme. Since we are using Workspaces Aware Webpack your build might be different depending what modules are you included in your store. More optional modules are coming in next major version so this number will be different.

BTW you shouldn't your build-ci script for building your theme. You should stick to just build script, build-ci script will be probably removed in next major. It's only there for github action test build across multiple os/node versions. I think that node scripts are documented well here.

I think it would be good to either stick to yarn or npm for the instructions and the lock file. Experienced users should be able to use the other one, anyhow.

Yes, I will drop yarn.lock in next major version.

Maybe it would be good to fail the build when needed modules are missing. At least it would have saved a day of searching :)

Hmmm if you are missing a module you shouldn't be able to enable your theme in first place. BTW with Workspaces Aware Webpack feature your are responsible for managing js/scss files for not used modules. If you disable or uninstall module webpack will still build those files and append theme to entry file.

cruftex commented 1 year ago

Hmmm if you are missing a module you shouldn't be able to enable your theme in first place. BTW with Workspaces Aware Webpack feature your are responsible for managing js/scss files for not used modules. If you disable or uninstall module webpack will still build those files and append theme to entry file.

I was building a release zip only with the required dependencies in package.json and then install it in the production shop. As a result, the modules are there to enable the theme, but JS code is missing, since during build there were no modules at all.

My mental model is thinking about distinct build, deploy and run steps and build and runtime dependencies being a separate thing. That was further supported by the fact that there is a released falcon.zip which contains everything needed.

Now I understand better how things work together at the moment. Thanks for the answer and your work for providing new technical base for a Prestashop Open Source theme.

I create another issue to discuss the webpack integration in more detail.

What about trying documenting the status quo a bit better for the moment? I would put these sections in the README:

Oksydan commented 1 year ago

Hi,

I am currently working on version 4.0 and new documentation page with gitbook. A lot of work has to be done but I would like to document as much stuff as possible.

ATM there is installation section https://github.com/Oksydan/falcon#installation that should answer most of your question tbh. I would like to simplify that installation process a bit in next version. I don't want to include every module as dependencies in release zip. I did this before and it forced me to release another version of the theme every time I did a small update to the module 😕.

cruftex commented 1 year ago

I don't want to include every module as dependencies in release zip. I did this before and it forced me to release another version of the theme every time I did a small update to the module .

Totally understand. It still should be documented what is in the release.zip, or, what is not. The problem is that users expect it to work the same way then other theme zips, but it does not.

Idea:

The other option would be to drop the release zip altogether.