add the csv distribution when dataset is a proxied csv #20
Conversation
| @@ -11,6 +11,7 @@ | |||
| "url": "git@github.com:koopjs/koop-output-dcat-us-11.git" | |||
| }, | |||
| "devDependencies": { | |||
| "@esri/arcgis-rest-feature-layer": "^3.2.1", | |||
There was a problem hiding this comment.
@drspacemanphd this is what I was talking about. This package is declared as a dependency in the example app, but when I run npm run dev the script chokes on a typescript error, saying that an interface from this package is not defined. Adding it as a dev and a peer dependency solves the issue and shouldn't affect consumers.
| const item = datasetToItem({ | ||
| id: dataset.id, | ||
| attributes: dataset | ||
| } as DatasetResource); | ||
| const requestOptions: IHubRequestOptions = { isPortal: false }; | ||
|
|
||
| return !!getProxyUrl(item, requestOptions); |
There was a problem hiding this comment.
Thinking out loud...
This set of conversions is clever, but squishy. We're basically taking a V3 API response, making it look sorta like a dataset, then making that sorta look like an item, then throwing it into the getProxyUrl method which expects a full item.
We can do this because we happen to know about the internals of getProxyUrl enough to know that this will work.
That said, if it works—great. A regression here would be low-impact and you have a test to validate it anyways.
One question though—why not just use isProxiedCSV instead of getProxyUrl?
There was a problem hiding this comment.
Okay—I see now that isProxiedCSV isn't part of the Hub.js public API. I'm not sure why—but you probably had a reason.
There was a problem hiding this comment.
@andrewctate thanks for the review!
Okay—I see now that
isProxiedCSVisn't part of the Hub.js public API. I'm not sure why—but you probably had a reason.
That was my original plan, but Tom is really trying to minimize hub.js' public footprint. I discussed it with him, but ultimately he asked for us to do it this way 🤷♂️
This set of conversions is clever, but squishy.
I know it's not ideal. Tom is doing a massive refactor for getting IHubContents that we'll probably use in the near future. The result of that refactor will already have the proxied csv stuff baked in 🙌
|
Thanks @andrewctate! |
Part of https://devtopia.esri.com/dc/hub/issues/2408
Imports proxied csv detection logic from hub.js
Also added
@esri/arcgis-rest-feature-layeras a dev and peer dependency to fix typescript errors when runningnpm run dev