Skip to content

.bs.js gets timestamp 1970-01-01 if .res had a warning #5206

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

Closed
cknitt opened this issue Jun 27, 2021 · 10 comments
Closed

.bs.js gets timestamp 1970-01-01 if .res had a warning #5206

cknitt opened this issue Jun 27, 2021 · 10 comments
Milestone

Comments

@cknitt
Copy link
Member

cknitt commented Jun 27, 2021

This just cost me almost a day to figure out. I was upgrading and refactoring a large (ReScript) React Native app, and suddenly Metro Bundler was not picking up any changes anymore until I restarted it with --reset-cache. Which I had to repeat every time I made a change. 🤯

The reason was that, in the process of refactoring, I had introduced some "unused open" warnings that I was planning to resolve later.

For the files with warnings, correct JS output was generated in the .bs.js, but the timestamp of the .bs.js was set to / remained 1970-01-01, so Metro did not refresh its cache and therefore the changes I made never became active in my app.

I think this is quite a serious issue and might affect other bundlers, too. I believe I have just not run into it before because I usually don't leave any warnings unresolved.

@bobzhang
Copy link
Member

Note the work around is turn warn-error.
This is a trade off to play. We set the time stamp so that the build system could always produce the same output (warnings are not flushed on a rebuild)

@cknitt
Copy link
Member Author

cknitt commented Jul 4, 2021

Ok, thanks for the information. I was already suspecting that this might be due to the "do not flush warnings" feature.

Still, the current workaround can lead to very unexpected behavior, and most probably not just with Metro.

Can you conceive of a different way to solve this problem that allows us to keep correct change dates for the JS output files with warnings?

@bobzhang
Copy link
Member

bobzhang commented Jul 5, 2021

will set the .cmj file instead, note it will trigger some unneeded rebuild, but this will only affect code who has warnings, so it may not be a big deal

@fhammerschmidt
Copy link
Member

I just got bitten by this again (and wasted more time than I'd like to admit) - can we expect it to be resolved soon?

@tjbo
Copy link

tjbo commented Dec 24, 2021

I got bitten by this... too.

I learned a lot about watchman and react native caching as the wrong timestamp was stopping "fast refresh" from working there.

I'd imagine it probably breaks other caching systems too.

@leoparis89
Copy link

leoparis89 commented Mar 22, 2022

I just got bitten by this again (and wasted more time than I'd like to admit) - can we expect it to be resolved soon?

Same here.
This really needs to be mentioned on the Rescript RN page because others are going to waste time trying to fix auto refresh.

@cknitt
Copy link
Member Author

cknitt commented Mar 29, 2022

It seems this also causes similar problems with Tailwind CSS (styles not being refreshed).

@cknitt
Copy link
Member Author

cknitt commented Jul 9, 2022

will set the .cmj file instead, note it will trigger some unneeded rebuild, but this will only affect code who has warnings, so it may not be a big deal

@bobzhang It would be great if you could look into this when you have some time. It would really make the lives of ReScript React Native developers a lot easier, and I would love to be able to include this in the 10.0 or 10.1 release.

@cknitt
Copy link
Member Author

cknitt commented Jul 21, 2022

In my desperation, I now tried to implement Bob's suggestion myself:

will set the .cmj file instead, note it will trigger some unneeded rebuild, but this will only affect code who has warnings, so it may not be a big deal

Indeed I managed to reset the timestamp of the .cmj instead of the bs.js when there were warnings, but it did not work as intended. Instead, it produced the error

rescript: error: '../bs/src/Test.cmj', needed by 'Test.cmj', missing and no known rule to make it

(Test.cmj is present, but has the 1970-01-01 timestamp set.)

Looking for some guidance here. @bobzhang @cristianoc

@cknitt
Copy link
Member Author

cknitt commented Oct 12, 2022

@bobzhang I am willing to invest some time here to get this finally fixed.

Before I dig into ninja, I would need some guidance on how to proceed. E.g., instead of the timestamp solution which seems problematic no matter which file's timestamp you modify, maybe some kind of additional "marker" file should be created for files with warnings?

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

No branches or pull requests

6 participants