repack icon indicating copy to clipboard operation
repack copied to clipboard

Bug in `invalidateScripts` leading to app crash

Open sriharshaarangi opened this issue 1 year ago • 4 comments

Environment

React Native android (but I believe the bug is not specific to RN android)

Description

Calling invalidateScripts for a chunk is causing a crash when next attempting to use that chunk. On debugging it, the chunk file is getting deleted, but the entry in sqlite db is not getting cleared. It looks like there is a bug in invalidateScripts method. resolveScripts method appends caller to the scriptId to get the cacheKey (const cacheKey = `${scriptId}_${caller ?? 'unknown'}`;), while invalidateScripts directly uses scriptId as the cacheKey.

Reproducible Demo

  1. Have a remote code split chunk.
  2. Call invalidateScripts with that chunk's scriptId
  3. Try using that chunk in UI, it leads to crash. com.facebook.react.common.JavascriptException: ChunkLoadError: Loading chunk <scriptId> failed

sriharshaarangi avatar Apr 16 '24 11:04 sriharshaarangi

@jbroma I am having few queries before getting into the fix:

  1. caller Documentation says Name of the calling script - it can be for example: name of the bundle, chunk or container. However, the default value is main which is not the default bundle/chunk/container name.
  2. Should default caller value be made main instead of unknown?
  3. I am unable to grasp what value the caller is adding for cacheKey in the first place, if the underlying downloaded file is the same irrespective of the caller value (I am assuming so as per the downloaded chunk file name). Can you please help elaborate, or point me to any other documentation behind the design decision?
  1. & 2. The name main comes from webpack, when you declare entry with a string or an array, it defaults the entry chunk name to main. See webpack docs. So it's a decent default I think, but it's good to double check this if this is really the case.
  2. I've looked at it and I'm pretty sure the caller is there to create some sort of uniqueID, since scriptID is a string and can easily be duplicated in an app. Adding a caller creates a scope for it so it's harder to get a collision.

jbroma avatar Apr 16 '24 21:04 jbroma

I suppose we could drop the caller altogether for the uniqueID, and rely on users specyfing output.uniqueName to avoid collisions.

EDIT: From my understanding, the scriptID is unique within a project, but there might be collision for example when using MF. If the configuration includes the output.uniqueName then we should be able to avoid any collision because the scriptID will be prefixed with uniqueName.

jbroma avatar Apr 17 '24 10:04 jbroma

This issue has been marked as stale because it has been inactive for 30 days. Please update this issue or it will be automatically closed in 14 days.

github-actions[bot] avatar May 18 '24 00:05 github-actions[bot]

This issue has been marked as stale because it has been inactive for 30 days. Please update this issue or it will be automatically closed in 14 days.

github-actions[bot] avatar Jun 26 '24 00:06 github-actions[bot]

This issue has been automatically closed because it has been inactive for more than 14 days. Please reopen if you want to add more context.

github-actions[bot] avatar Jul 10 '24 00:07 github-actions[bot]