Potential work-around for Rollup cyclic dependency bug #2332
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
There is a bug in Rollup that non-deterministically fails to detect cyclic
dependencies. rollup/plugins#1425
As a result
lru-cache
is sometimes wrapped in arequireLruCache
function and sometimes it is not.This makes our build non-deterministic.
lru-caceh
is not itself part of a circular dependency butsemver/classes/range.js
, which depends onlru-cache
, is.By depending on
lru-cache
directly from our app, we change the order in which modules are analyzed by Rollup, hopefully avoiding the buggy behavior.It probably doesn't matter where we put this import, so I put it as close to the root of the app as I could find.
Changes
Add
import "lru-cache"
in frontend/src/routes/+layout.tsTests
Inspected generated
vendor
chunk to see that it does not havehasRequiredLruCache
.dfx deploy nns-dapp
locally and did some manual testing.