amaannawab923 commented on code in PR #45522:
URL: https://github.com/apache/superset/pull/45522#discussion_r4401413204
##########
superset-frontend/webpack.config.js:
##########
@@ -211,6 +211,24 @@ const plugins = [
singleton: true,
eager: true,
},
+ // Shared for two reasons. First, the registries in here hold their
+ // state in module-level closures, so a second copy of the module is a
+ // second, unreachable registry: an extension calling
+ // `getExtensionsRegistry().set(...)` would write somewhere the host
+ // never reads, and the call would look like it worked.
+ //
+ // Second, this is where `SupersetClient` lives. Without it an extension
+ // that talks to Superset's API has to hand-roll `fetch`, including
+ // fetching and attaching a CSRF token for every mutation -- which is
+ // most of what `SupersetClient` exists to do.
+ //
+ // The host loads this module on every page already, so providing it
+ // eagerly costs no extra bytes and keeps extensions off an async
+ // boundary to reach it.
+ '@superset-ui/core': {
Review Comment:
Agreed on both counts, and fixed in f4a38f7 by dropping the share rather
than documenting it.
Scoped it against what extensions actually use first. Across our own
extension the entire surface is three verbs and two options:
`SupersetClient.get/post/put({ endpoint, jsonPayload })`, with only `json` read
off the result. So the contract we were implicitly signing was enormous
relative to the contract anyone wanted.
`@apache-superset/core` now has an `http` namespace -- `get`, `post`, `put`,
`delete`, each taking an `endpoint` and returning the parsed `json`, the
`status` and the response `headers`. The host side is a thin adapter over
`SupersetClient`, so the transport is unchanged and only the contract is new: a
named surface we can keep stable while the package behind it moves.
The endpoint must be a path under this Superset. An absolute URL is refused,
and so is a protocol-relative one -- `//host/path` resolves to another origin
while still beginning with a slash, which my own test caught before the guard
did. The call carries the session and the CSRF token, so reaching another
origin is `fetch`'s job.
The share is gone from the host config and from the scaffold, which also
settles the scaffold half of the `@superset-ui/core` declaration thread: it
stops mentioning the package rather than starting to declare it. Docs and
UPDATING no longer point authors at `SupersetClient` and say what happens if
they import it anyway.
One thing worth surfacing from the scoping, since it argues the same way:
`@superset-ui/[email protected]` on npm was published 2024-12-10 and has no `exports`
map and no `types` field, while the in-repo 0.20.4 is subpath-exports based.
Same version string, structurally different packages, and nothing can warn
about it because the versions match and neither side declares
`requiredVersion`. That is a sharp edge the share was handing to every
extension author, independently of how large the surface was.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
