exaco / laravel-octane-dockerfile

Production-ready Dockerfile for Laravel Octane (FrankenPHP, Swoole, RoadRunner) powered web services and microservices. Done right.
MIT License
590 stars 89 forks source link

Feat/vite (front end) build #43

Closed sloan58 closed 1 year ago

sloan58 commented 1 year ago

If npm dependencies exist (package.json or package-lock.json), build front end assets.

smortexa commented 1 year ago

@sloan58 The problem still exists. The tests fail.

sloan58 commented 1 year ago

That's interesting. If you check the actions that ran when I pushed yesterday they all passed. I see that you adjusted the npm run build section. Was that before or after the tests ran?

sloan58 commented 1 year ago

I think we need to leave this in there. If there's no package.json file, we shouldn't build. My local tests fail as well on the current codebase if I delete package.json

RUN if [ -f $ROOT/package.json ] || [ -f $ROOT/package-lock.json ]; \
  then \
  npm run build; \
  fi
smortexa commented 1 year ago

I agree with you. After making the adjustments, but the tests failed. The if command was preventing the npm run build command from executing.

sloan58 commented 1 year ago

With my latest commit I was able to build an image with or without a package.json file. Maybe there should be two additional tests (two with and two without package.json for swoole and rr)?

smortexa commented 1 year ago

That's true. But I removed it to test that the build stage works correctly.

sloan58 commented 1 year ago

I guess I'm not sure why we updated the if statement. My local tests and the Github actions passed with my latest commit, and if we do not build the frontend assets, they'll still get copied over to the final image since we're running a COPY . . regardless during the build stage.

smortexa commented 1 year ago

By using the if statement, the image can be built successfully without encountering any errors. However, it's important to note that the npm run build command will not be executed, which means that our front-end assets will not be built even though the image is successfully built.

sloan58 commented 1 year ago

Understood. If there's no package.json file we shouldn't run npm run build as it will fail. If we want to test the case that package.json doesn't exist, we could have two more tests that delete package.json prior to building the image.

smortexa commented 1 year ago

Ahhhhhh, the package-lock.json and package.json files are listed in .dockerignore file!

sloan58 commented 1 year ago

Awesome! Thanks for the help getting this merged!