Skip to content
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

feat(remix-dev): only purge require cache if updated #2046

Closed
wants to merge 4 commits into from

Conversation

warmpigman
Copy link

Fixes #1818 Check version of assets.json, if it has changed, then purge require cache.

@warmpigman warmpigman changed the title Dev Only purge require cache if updated Feb 19, 2022
@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Feb 21, 2022

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@MichaelDeBoey MichaelDeBoey changed the title Only purge require cache if updated feat(remix-dev): only purge require cache if updated Jun 3, 2022
@MichaelDeBoey
Copy link
Member

@warmpigman Could you please rebase your branch onto latest dev & resolve conflicts?

@MichaelDeBoey MichaelDeBoey added the needs-response We need a response from the original author about this issue/PR label Jun 3, 2022
@warmpigman
Copy link
Author

@warmpigman Could you please rebase your branch onto latest dev & resolve conflicts?

@MichaelDeBoey I think I did so, could you check?

@github-actions github-actions bot removed the needs-response We need a response from the original author about this issue/PR label Jun 9, 2022
for (let key in require.cache) {
if (key.startsWith(buildPath)) {
delete require.cache[key];
if (require.cache[buildPath + "/assets.json"]?.exports.version !== JSON.parse(require('fs').readFileSync(BUILD_DIR + '/assets.json')).version) {
Copy link
Member

Choose a reason for hiding this comment

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

Could we make this an early return please?

This will create a positive condition (which has a lower cognitive load) + will make sure the code is less nested.

@dmarkow
Copy link
Contributor

dmarkow commented Jun 10, 2022

I'm excited to see this get fixed, but does this approach still work? As far as I can tell, Remix stopped creating that assets.json file as of v1.2.0.

@balzdur
Copy link

balzdur commented Jan 9, 2023

What about this approach ?

@pcattori
Copy link
Contributor

Superceded by #5133

@pcattori pcattori closed this Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants