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

Add lazy/eager cache key to avoid invalid change when switching modes #9518

Merged
merged 2 commits into from
Feb 2, 2024

Conversation

JakeLane
Copy link
Contributor

@JakeLane JakeLane commented Feb 1, 2024

↪️ Pull Request

Simple change to consider whether lazy mode is enabled/disabled. This is required because launching with --lazy, then re-launching without --lazy will cause build errors.

🚨 Test instructions

  1. yarn parcel --lazy
  2. Shut down server
  3. yarn parcel
  4. Observe no build error

@JakeLane
Copy link
Contributor Author

JakeLane commented Feb 1, 2024

There's also a cache key in packages/core/core/src/UncommittedAsset.js, which I did not change. Let me know if this is acceptable, I assumed there was no need but my mental model may be incorrect.

Copy link
Contributor

@marcins marcins left a comment

Choose a reason for hiding this comment

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

Nice! Was the approach just to find everywhere that mode was used in a cache key and add it there?

@@ -665,7 +665,8 @@ export default class PackagerRunner {
bundleGraph.getHash(bundle) +
JSON.stringify(configResults) +
JSON.stringify(globalInfoResults) +
this.options.mode,
this.options.mode +
`${this.options.shouldBuildLazily ? 'lazy' : 'eager'}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

/nit but this doesn't really need to be a template string.. (but I get that you probably just copied what you did in all the other places, no big deal..)

@@ -6788,4 +6790,43 @@ describe('cache', function () {
let res = await run(build.bundleGraph);
assert.deepEqual(res, {default: 'foo'});
});

it('invalidates correctly when switching from lazy to eager modes', async function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this was a failing test before your changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, it was!

@JakeLane
Copy link
Contributor Author

JakeLane commented Feb 1, 2024

Nice! Was the approach just to find everywhere that mode was used in a cache key and add it there?

Yep, I also looked at the Parcel version variable

@JakeLane JakeLane force-pushed the jlane2/fix-dirty-cache-switch-lazy-mode branch from 0aa9f8f to a391e4b Compare February 2, 2024 00:41
@JakeLane JakeLane merged commit 4debc2e into v2 Feb 2, 2024
14 of 16 checks passed
@JakeLane JakeLane deleted the jlane2/fix-dirty-cache-switch-lazy-mode branch February 2, 2024 02:27
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.

3 participants