observablehq / runtime

The reactive dataflow runtime that powers Observable Framework and Observable notebooks
https://observablehq.com/@observablehq/how-observable-runs
ISC License
1.01k stars 71 forks source link

Global Namespace Pollution #241

Closed GordonSmith closed 4 years ago

GordonSmith commented 4 years ago

I suspect this is being pulled in by d3-require, but window.define gets overwritten even if it existed prior to loading this library: https://github.com/d3/d3-require/blob/master/src/index.js#L88

mbostock commented 4 years ago

That’s correct. Unfortunately this is not something we can fix because it’s how AMD works: the global define is needed so that when require’d modules load asynchronously, they can register themselves with the AMD loader.

mbostock commented 4 years ago

Duplicate of d3/d3-require#33.

GordonSmith commented 4 years ago

@mbostock This is bit of a shame as it prevents the embedding of download notebooks in third party web sites...

mbostock commented 4 years ago

@GordonSmith It only applies if you call require, in which case you’ll need the global define or the notebook will be broken, anyway. If you don’t call require, the global define won’t be set.

GordonSmith commented 4 years ago

Unfortunately that is any notebook which contains some markdown as the stdlib requires the markdown js library,..

IOW embedding any notebook which contains markdown will overwrite the previously declared global define, potentially causing pain for the hosting website.

To be fair AMD require implementations have been battling this issue forever...

mootari commented 4 years ago

@GordonSmith I recommend moving the discussion to https://talk.observablehq.com/, where others with practical experience wrt embedding might be able to assist you.

As an aside, you could try to define get/set callbacks for window.define (here are some examples for scope wrapping and object proxying).