https://serverless-stack.com/ logo
Join Slack
Powered by
<@U01MV4U2EV9> after finalizing the GraphQL stack,...
# general
f
@thdxr after finalizing the GraphQL stack, we can rework the 
GraphqlApi
construct and remove 
graphql
from the dependency right? (so we don’t get pooped again @Derek Kershner 🥲)
t
CDK's synchronous requirement breaks all ability for us to have optional dependencies
so I don't know at the moment how to mark certain dependencies as option when we use them in a construct since import(dynamic) requires await
We have to figure out a strategy for this because our deps are getting heavy
d
its a powerful emoji to cause such discussion! 😁
More seriously, the combo of graphql being both: 1. Unable to handle multiple versions of itself. 2. Unable to be optional. Presents a real situation. Maybe the graphQL construct should just be a separate package? 🤷
Or…what’s wrong with peer? (with like 
>=15
)
t
yeah it's a bit complicated so I'll lay it out (and I might have some more research to do) in ESM if you have an index file
Copy code
export * from "./GraphQLConstruct"
export * from "./NonGraphQLConstruct"
And someone imports anything from the index file, all the imports from all files are evaluated (need to verify this but the errors people posted on the previous build suggests this is true) So that means it'll try to evaluate 
import graphql from "graphql"
in 
GraphQLConstruct
even if you just import 
NonGraphQLConstruct
This is generally fine because if it's truly an optional dependency you should be using a dynamic import
Copy code
const graphql = await import("graphql)
I can't do this inside a construct because CDK is class based all construct code needs to be synchronous I need to see if doing it outside the construct avoids the issue above
if that doesn't work, we might need to shift to more explicit import paths (which is the direction ESM is headed anyway) so you'd do 
import NonGraphQLConstruct from "@serverless-stack/resources/NonGraphQLConstruct"
yep verified all of this. The only way to get optional imports is by not having them top level which CDK constructs do not allow you to do 😢
We could dynamically import it at top level if we can figure out a condition to check
Copy code
const graphql = someCondition ? await import(...) : undefined
this absurd hack works
Copy code
import { createRequire } from "module"
const require = createRequire(import.meta.url);
let hasGraphQL = false
try {
  require.resolve("graphql");
  hasGraphQL = true
} catch { }
const { print } = hasGraphQL && await import("graphql")
const { mergeTypeDefs } = hasGraphQL && await import("@graphql-tools/merge");
const { loadFilesSync } = hasGraphQL && await import("@graphql-tools/load-files");
this also works
Copy code
async function weakImport(pkg) {
  try {
    return await import(pkg)
  } catch {
    return {}
  }
}
const { print } = await weakImport("graphql")
const { mergeTypeDefs } = await weakImport("@graphql-tools/merge");
const { loadFilesSync } = await weakImport("@graphql-tools/load-files");
I guess this will be what we have to do
OK I got rid of all the peer dep warnings when installing
d
Ugly, but I still think it's better than having as a hard dep.
t
yeah agree
hence 1.2.2
d
I appreciate the extra work to alleviate us downstream folks as well. I know that had been a journey, but it allows me to recommend SST more strongly.
j
🔥 @thdxr on that dependency warning hack.
t
ultimately I blame CDK
as usual
d
It wouldn't be software engineering without dependency blame.
Open in Slack
PreviousNext
Powered by