Using Plot with Webpack can lead to ReferenceErrors ("temporal dead zone") in production.

[parkerziegler]

Hi folks! Thanks for all of your hard work on Plot, I've really been enjoying using the library and learning more about the philosophy behind its design! This bug report comes from an issue I encountered while building a small Next.js application using some of the APIs from Plot.

Minimal Reproduction

To make the bug easier to see, I created a minimal reproduction. The README has instructions for how to run the project and see the errors; getting going just involves running two yarn commands. Let me know if anything is unclear in how to get the reproduction working.

Bug Description

When serving a production build of the minimal reproduction above, we hit the following runtime error:

ReferenceError: can't access lexical declaration 'd' before initialization (Firefox)
ReferenceError: Cannot access 'd' before initialization (Chrome)

After doing some digging using production source maps, I believe the core issue is that Plot is trying to extend from the Mark class in facet.js before Mark has been declared in the built Webpack chunk.

Running a development build of this project does not throw any runtime errors.

Stack Trace

The stack trace of the reported error is as follows (Chrome):

ReferenceError: Cannot access 'd' before initialization
    at Object.vc (497-35515fda497bb06a.js:1:665)
    at Object.27 (facet.js:13:21)
    at r (bootstrap:21:23)
    at Object.2186 (497-35515fda497bb06a.js:1:1422)
    at r (bootstrap:21:23)
    at Object.3962 (histogram-6827f696cfad69e9.js:1:297)
    at r (bootstrap:21:23)
    at (index):5:16
    at route-loader.js:236:51

Following the stack trace is a bit difficult since the production build is minified. However, I did turn on source maps using productionBrowserSourceMaps: true in next.config.js in the reproduction. This is what allows us to trace that the error arose from L13, C21 in facet.js.

What does the stack trace suggest?

L13, C21 of facet.js pertains to the Mark class imported from mark.js. The issue appears to be that, at the time class Facet is being initialized to extend Mark, Mark itself has not yet been initialized. This is what leads to the ReferenceError. d in the minified build pertains to class Mark.

These types of ReferenceErrors in production Webpack builds tend, based on this comment from Webpack's core author Tobias Koppers, to be caused by cyclic dependencies.

Discussion

Does @observablehq/plot have cyclic dependencies?

Yes. Adding the no-cycle rule from eslint-plugin-import to @observablehq/plot resulted in 52 detected cycles.

To be clear, cyclic dependencies are allowed by the ESM spec; they can just (under some circumstances) lead to these kinds of temporal deadzone errors in Webpack. In addition, tree shaking in Webpack makes this even more tricky to track down, because, depending on which parts of the Plot API you use, the chunk Webpack generates for @observablehq/plot will resolve these cycles in different orders. This means you may use a set of APIs that result in a perfectly fine production build with no runtime errors.

What could be done to alleviate this error?

The solution with the highest probability of success would be to eliminate all cycles from the source. However, this is likely prohibitively expensive due to how many cycles currently exist and the challenges associated with such large-scale refactoring.

The specific error here appears to be the result of the following cycle:

• facet.js imports Mark from mark.js
• mark.js imports plot from plot.js
• plot.js imports facets from facets.js

Resolving this cycle could help to alleviate this problem, but I'm not fully certain. It appears to be the most likely candidate based on the stack trace, but I have noticed that this error appears to be mark type dependent. For example, in another project where I use the Plot.barY and Plot.dot marks APIs in the same project, I don't encounter this error. This suggests that the troublesome cycle may be related to some of the transforms APIs (e.g. binX, normalizeX) that the histogram and strip plots use.

Additional Snags

One interesting note is that, if you remove either of the charts in the minimal reproduction (e.g. by deleting histogram.js or strip-plot.js), the error disappears on the page that is left behind. I'm still trying to understand how this affects the resolution of cycles in the built chunk that includes the Plot source.

Final Thoughts

I recognize this may not be a high priority fix for the team, and that using Plot in Webpack-based applications in general may be outside of the core use case of the library. It also may well be that this bug is better directed to the Next or Webpack teams to address; I'm happy to escalate it up to them if you all agree. I mostly opened this bug because I spent a fair bit of time (out of my own curiosity) hunting down its origins myself and wanted to share in case it's helpful to the team, or in the event that someone else encounters this same issue. I'd be happy to try to come up with a draft solution as well, it may just be quite slow with the semester about to get underway for me.

Again, thank you for all the hard work on Plot and I hope to keep contributing!

[mbostock]

It also may well be that this bug is better directed to the Next or Webpack teams to address; I'm happy to escalate it up to them if you all agree.

JavaScripts tools often fail to process valid JavaScript correctly (see d3/d3-array#147 and its duplicates for a popular example). I don’t find it practical, and certainly not enjoyable, to keep in mind what subset of valid JavaScript will make currently popular tools happy—I’d rather just learn and conform to the language specification. So certainly I would prefer if the tools fixed themselves by preserving the semantics of valid JavaScript, and that folks didn’t try to shift that onus onto library authors. 😉

That said, I don’t feel like we strictly need cyclical imports in Plot, and that it’s mostly happenstance. And having ESLint catch when we introduce cycles would reduce the burden of avoiding cyclical imports, or at least amortize it. Though certainly it would introduce some burden on us, again for the benefit of tools that cannot support the specification.

So, I’ll keep this open mostly because I appreciate how much effort you put into describing the problem. And at some point I’ll try to remove the cycles. But I might also decide that it’s not worth the effort, and so if your immediate objective is to get this fixed it’s probably worth pursuing other options in parallel.

[parkerziegler]

Thanks for the reply @mbostock, I really appreciate hearing your perspective! I wholeheartedly agree that library authors shouldn't be responsible for keeping in mind the restrictions of end users' build tools, particularly if it's the build tools that can't support the spec. It's even difficult to debug issues like this because understanding how cycles get resolved in chunks seems to require a much deeper understanding of Webpack than I have (or care to acquire 😅).

But I might also decide that it’s not worth the effort, and so if your immediate objective is to get this fixed it’s probably worth pursuing other options in parallel.

Completely understandable, and it's not an immediate (or even long term) objective of mine to have this fixed 😄 Hopefully this issue can serve as a pointer if other folks come across a similar problem and save you some extra key strokes. Feel free to close this at any point that feels right for you and the team 👍

[JoeDuncko]

Hi! I think I'm actually seeing this bug in development using Create React App 5.0.0.

I encountered it when I went to add a Plot.legend(). Plot.plot() was working like a charm in other files up until this point. Removing the Plot.plot()s did not seem to fix the errors, as some notes in the OP might suggest.

After reading this issue (thanks so so much for the writeup @parkerziegler !) I came up with a no-good-very-bad "solution": calling Plot.plot({}); immediately before my Plot.legend().

I thought I'd post this here to offer it as a stopgap for anyone else encountering this issue, and get feedback to see if there's a less hacky solution that can be used instead.

Thanks!

Stack trace for reference:

Uncaught ReferenceError: Cannot access 'Mark' before initialization
    at Module.Mark (swatches.js:104:1)
    at Module../node_modules/@observablehq/plot/src/facet.js (facet.js:13:1)
    at Module.options.factory (react refresh:6:1)
    at __webpack_require__ (bootstrap:24:1)
    at fn (hot module replacement:61:1)
    at Module../node_modules/@observablehq/plot/src/plot.js (mark.js:346:1)
    at Module.options.factory (react refresh:6:1)
    at __webpack_require__ (bootstrap:24:1)
    at fn (hot module replacement:61:1)
    at Module../node_modules/@observablehq/plot/src/mark.js (swatches.js:104:1)

[mbostock]

I think I'm actually seeing this bug in development using Create React App 5.0.0.

Sorry to be pedantic, but to be clear: this is a bug in Webpack (and possibly other JavaScript bundlers) that fail to preserve the semantics of ES modules with respect to circular imports. This is not a bug in Plot.

We may workaround the limitations of popular tools, but if you want to help fix this problem, it would be more productive to discuss this in and/or contribute to the webpack repo (or create-react-app, or whichever tool you are using) that is the source of this bug. Discussing it here means that the people responsible for this bug will be less aware of the downstream problems it is causing.

[mbostock]

Related #619.

[JoeDuncko]

I think I'm actually seeing this bug in development using Create React App 5.0.0.

Sorry to be pedantic, but to be clear: this is a bug in Webpack (and possibly other JavaScript bundlers) that fail to preserve the semantics of ES modules with respect to circular imports. This is not a bug in Plot.

My bad, definitely didn't mean to use the term "bug" to place fault or expectations onto Plot or the Plot team. Based on the above discussion, I have zero expectations for this to be resolved from the Plot side. I was just looking for a word to refer to the problem, and I should have used the term "error" instead. 👍