Skip to content

Avoid CLI re building the project if it's already built #187

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

Merged
merged 7 commits into from
Jun 29, 2025

Conversation

gboutte
Copy link
Contributor

@gboutte gboutte commented Jun 28, 2025

What does it do?

  • The CLI check is the project have already been built, and does not rebuild it
  • Migrated the playground to typescript

Why is it needed?

In some cases the strapi project is deployed in a readonly mode, and the project is already built. So if we run the CLI in the environment we will get permission error to write.

How to test it?

In a typescript project, the first CLI run will build the project (if not alrady built). To verify that it's not re building we can manualy change something in the dist folder, and re run the CLI. Since the dist folder already exist, it won't rebuild.

Related issue(s)/PR(s)

Fix issue #186

@boazpoolman boazpoolman requested review from Copilot and boazpoolman and removed request for Copilot June 28, 2025 16:36
Copilot

This comment was marked as off-topic.

Copy link
Member

@boazpoolman boazpoolman left a comment

Choose a reason for hiding this comment

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

Nice work @gboutte!

Thank you for migrating the playground to TypeScript together with solving #186.

The code change looks good, it's just that the test you've wrote seems to be breaking the e2e (Cypress) tests which run after the CLI tests. I've debugged it a bit and it seems the fact that you (re)move files in the dist folder causes the dist to be corrupted after the tests have ran.

Could you take a look at that? Maby you could find out a different way to run these tests. Or if that doesn't work, maby you can try to build the playground again, after running the integration tests, restoring the dist folder.

@gboutte
Copy link
Contributor Author

gboutte commented Jun 29, 2025

I looked at this problem.
Locally I tried to run yarn playground:start after running the tests and indeed the admin is not working I get a webpage showing Not Found and in the server console I get

 Error: ENOENT: no such file or directory, open '/home/greg/dev/strapi-plugin-config-sync/playground/node_modules/@strapi/admin/dist/server/server/build/index.html'

I did some tests by removing my changes and my tests, and if I run yarn playground:build before the tests and the start the admin panel works fine. But if I delete the dist folder and run the tests and then the start. The test will compile the app and create the dist folder. But the stat will give the same error.

I think that the strapi build command is not giving the same output as the compileStrapi method.

I don't know if it's possible to make both result the same.

I made a fix on the tests so that the dist folder is restore a the end. It should run normally now.

Copy link

codecov bot commented Jun 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (master@8a7d4ba). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             master      #187   +/-   ##
==========================================
  Coverage          ?   100.00%           
==========================================
  Files             ?         2           
  Lines             ?        22           
  Branches          ?         3           
==========================================
  Hits              ?        22           
  Misses            ?         0           
  Partials          ?         0           
Flag Coverage Δ
unit 100.00% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@boazpoolman boazpoolman left a comment

Choose a reason for hiding this comment

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

It's interesting that strapi build and compileStrapi don't give the same output in the dist folder.

Anyways, this solution is fine for the scope of this issue. Thanks again! I'll get this released asap.

@boazpoolman boazpoolman merged commit 748a31a into pluginpal:master Jun 29, 2025
6 checks passed
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

Successfully merging this pull request may close these issues.

2 participants