[RFC & POC] Hiding Wasp's private API from users #1922
Conversation
sodic
changed the title
sodic
force-pushed
the
filip-hide-private-api
branch
from
8f58277
to
0b19844
Compare
| @@ -30,6 +30,7 @@ const defaultViteConfig = { | |||
| outDir: "build", | |||
| }, | |||
| resolve: { | |||
| conditions: ["client-runtime"], | |||
There was a problem hiding this comment.
This is where we set the framework runtime's condition string, thus gaining access to the SDK's private API.
| "./client/operations/queryClient": { | ||
| "client-runtime": "./dist/client/operations/queryClient.js" | ||
| }, |
There was a problem hiding this comment.
This how we expose the SDK's private API to the framework, while hiding it from the user.
Unfortunately, we can't do:
"exports": {
"client-runtime": {
// private API
},
"default": {
// public API
}Because once Vite finds an object with a matching condition, it won't trace back if it doesn't find a required symbol inside it:
Vite has a list of "allowed conditions" and will match the first condition that is in the allowed list.
Which means we would have to repeat the entire public API twice.
"exports": {
"client-runtime": {
// private API
// public API
},
"default": {
// public API
}It isn't quite as nice, but works. We can still decide which approach do we want to take.
There was a problem hiding this comment.
Great stuff! I didn't know that exports API worked this way. I always thought that the import and require were the only options.
There was a problem hiding this comment.
I've reorganized the index file to include export only the public API.
| import { useQuery } from '../client/operations' | ||
| import { addMetadataToQuery } from '../client/operations/queries/core' |
There was a problem hiding this comment.
I've updated all the import paths inside the SDK have been converted from package imports to relative imports (also accounting for the updated paths).
| @@ -7,7 +7,7 @@ import router from './router' | |||
| import { | |||
| initializeQueryClient, | |||
| queryClientInitialized, | |||
| } from 'wasp/client/operations' | |||
| } from 'wasp/client/operations/queryClient' | |||
There was a problem hiding this comment.
All SDK imports in the framework code have been updated according to the new code organization.
|
Nice job @sodic , nice analysis! It all sounds reasonable, and it is cool that there is such thing as conditional exports. How much is our solution "hacky" here -> is this what conditional exports are supposed to be used for? What about that potential bug with symlink you mentioned -> did you get any response regarding that? Additional idea, I will shortly explain it and you tell me if it doesn't make any sense: what if we had another package, called "wasp-core", that would contain logic from SDK that is used both by SDK and the framework code? And then that package would be used by SDK, but not reexported, and it would also be used by framework code. We would need to do refactoring, extract code from SDK into it, but result would achieve what we wanted, right? What would the pros/cons be here (if it works)? Btw I will let @infomiho do the details review of this PR, while I will stick to high level stuff. |
Is this hacky?
Not very hacky. Conditional exports are meant to be used with user-provided strings and arbitrary semantics (read the docs for details). Of course, I doubt anyone envisioned our case specifically (since we're so cutting edge :)), but it's not like we're abusing a completely unrelated mechanism. How does it compare to the
|
Ok! Convincing analysis, makes sense to me. I am ok with sticking with the approach you started in this PR, and it is nice to know we potentially also have some other options in the future also on the table. |
1 Context
The plan for Wasp 0.12.0 was:
node_modules/wasp) and expose it to the user..wasp/out{server,web-app}), hiding it from the user.Unfortunately, due to tight coupling between private and public functionalities, we've ended up having to move a large chunk of Wasp's private API into the SDK:
2 The private API problem
The framework code (i.e., the runtime) importing private API from the SDK is not a problem in and of itself. However, it does requires the SDK to export the private API, making it no-longer-private.
In other words, Wasp's private API is visible to the user.
Perhaps even worse, Wasp's private API is also visible to the IDE which suggest it thorough autoimports, further nudging the user to rely on undocumented or incomplete functionalities.
3 Possible approaches
I've identified two distinct approaches for making Wasp's private API truly private:
In simpler terms: just move all the private stuff to the framework code and make sure the SDK doesn't need it.
While obvious and seemingly the best solution, it does have its downsides - it's very time consuming, risks breaking something in Wasp, and would cause duplication whenever a public function shares implementation details with something from the framework code.
If possible, doing so should give us an identical visible result with much less work and no duplication.
This PR proposes a way to implement the second approach.
4 The proposed solution
As a Proof of Concept, I've implemented all the proposed changes in a single "module" of our SDK (
wasp/client/operations).An SDK module exposes each symbol for one or more of the following reasons:
Important
We want to visibly expose only the first group: The imports our users need.
Fixing the second group of exposed imports (required by the SDK) is simple and only requires changing import paths.
Fixing the third group of exposed imports (required by the framework) is more complicated, but absolutely possible.
4.1 Code organization prerequisites
For the proposed approach to work, we must first do some reorganization:
wasp/client/operations/index.ts) exposes the public API and nothing else.Do this by removing all internal (SDK or framework) re-exports and relocate all direct exports.
Done here.
Turn package imports into relative imports where necessary. For example,
import x from 'wasp/client/operations'becomesimport x form '../../client/operations'.This removes the need to expose imports needed in the SDK.
Done here.
Done here.
4.2 Hiding the private API
The above 3 steps remove the need to export private API used exclusively in the SDK and do some reorganization, but we're not done yet.
The main question has always been: How do we expose SDK's private API to the framework without exposing it to the user too?
This is where conditional exports come in:
The solution is to group the private API paths under a condition string (done here) that we only set when running our framework code (done here).
If we decide to implement this RFC, server and client will be setting different condition strings.
4.3 The only remaining problem
I don't want to be the "there's a bug in the compiler" guy again, but...
The setup I've described here works flawlessly... for packages that aren't local symlinks.
With our SDK (that is a local symlink), this happens:
initqueryclient.mp4
I can't get TypeScript's language server (TLS) to ignore the folder inside the
.waspdirectory.Based on feedback I got from
npx tsc --traceResolution, I'm guessing this happens because TLS knows about the packagenode_modules/wasp, but the folder being a symlink confuses TSL, which then starts treating the symlink's source folder as part of the user's code.When I copy or hardlink the SDK into node modules, this doesn't happen. I'll report a bug to someone and see what they say.