Skip to content

Commit

Permalink
Potential work-around for Rollup cyclic dependency bug (#2332)
Browse files Browse the repository at this point in the history
# 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 a `requireLruCache`
function and sometimes it is not.
This makes our build non-deterministic.
`lru-caceh` is not itself part of a circular dependency but
`semver/classes/range.js`, which depends on `lru-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.ts

# Tests

Inspected generated `vendor` chunk to see that it does not have
`hasRequiredLruCache`.
`dfx deploy nns-dapp` locally and did some manual testing.
  • Loading branch information
dskloetd committed Apr 19, 2023
1 parent 027bd72 commit 51e4e1f
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions frontend/src/routes/+layout.ts
@@ -1,3 +1,16 @@
// There is a bug in Rollup that non-deterministically fails to detect cyclic
// dependencies. https://github.com/rollup/plugins/issues/1425
// As a result 'lru-cache' is sometimes wrapped in a require* function and
// sometimes it is not. This makes our build non-deterministic.
// lru-caceh is not itself part of a circular dependency but
// semver/classes/range.js, which depends on lru-cache, is.
// By depending on lru-cache directly from our app, we change the order in which
// modules are analyzed, 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.
// TODO: Remove when the Rollup issue is fixed.
import "lru-cache";

export const prerender = true;

// TODO: no ssr for local development until https://github.com/dfinity/ic-js/issues/238 solved
Expand Down

0 comments on commit 51e4e1f

Please sign in to comment.