search

observablehq / stdlib

The Observable standard library.
https://observablehq.com/@observablehq/standard-library
ISC License
966 stars 83 forks source link

File attachment URL strings (3.3.2) #172

Closed jashkenas closed 4 years ago

jashkenas commented 4 years ago

Ref: https://github.com/observablehq/notebook/issues/5002 Previously: https://github.com/observablehq/compiler/pull/24 (Private issues)

If a FileAttachment object is created with a URL object, ensure that it is coerced to a string.

mbostock commented 4 years ago

I feel it would be more appropriate to do the coercion in the FileAttachments method (rather than the constructor), since that’s where we coerce name to a string. Or, we could coerce both name and url to strings in the constructor, but that’s redundant with the coercion already happening in the FileAttachments function (which is also the only place that this constructor is ever called).

jashkenas commented 4 years ago

I’ve updated the PR to move the coercion to the url() function instead. I looked closer at the comment that reminds us that the FileAttachments resolve function may resolve URLs asynchronously. We should only await that promise at the point where the .url() function is finally being called.

Plus a test.