-
Notifications
You must be signed in to change notification settings - Fork 507
fix: don't remove dist folder, wipe content instead #272
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
Conversation
|
@grimnomad can you test if this fixes the issue? |
|
I tested the fix by manually altering the index.js on my machine. I don't know how to get this PR working with my project. But it doesn't work. CRA still fails, because there is no file at the time it tries to access the file. This is what CRA prints: |
|
Bummer, I wonder if this is a different issue then. Could you make an example lerna repo where I could test this out? As mentioned in the other thread, this could just be a race condition between tsdx and CRA where tsdx is cleaning the folder and CRA is trying to access it at the same time. I suppose we could add a |
|
https://github.com/grimnomad/lerna_tsdx This is a minimal example. From my point of view it is a race condition. A |
|
I am starting the watch task not the build task. In my opinion the missing module is a consequence of the "error". If I start both tasks manually (first common, than web) everything is ok. If I start them via lerna in parallel I get the module error. I tested the build steps |
|
Is this ready? |
|
This change does not fix grimnomads issue, we can make this change if we want, it at least keeps the dist folder in existence if something else is relying/watching for it. Grim's issue is that he is running a tsdx build process and a CRA start process at the same time, so CRA is trying to use files from another subpackage's dist folder and they are being moved/modified. I dont think there is a solution that we can provide for this, lerna would need to provide a way to sequentially order package scripts. The |
|
@audiolion |

Potentially fixes #260