-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add exports for constructs #2483
base: main
Are you sure you want to change the base?
Conversation
|
65dda86
to
f0924e2
Compare
"./lib/constants/access": "./lib/constants/access.js", | ||
"./lib/constants/context-keys": "./lib/constants/context-keys.js", | ||
"./lib/constants/fastly-aws-account-ids": "./lib/constants/fastly-aws-account-ids.js", | ||
"./lib/constants/library-info": "./lib/constants/library-info.js", | ||
"./lib/constants/metadata-keys": "./lib/constants/metadata-keys.js", | ||
"./lib/constants/regex-pattern": "./lib/constants/regex-pattern.js", | ||
"./lib/constants/ssm-parameter-paths": "./lib/constants/ssm-parameter-paths.js", | ||
"./lib/constants/tracking-tag": "./lib/constants/tracking-tag.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our modules aren't super consistent right now, sometimes they're a single file and sometimes they're a directory with an index.js.
I'd like to avoid a big refactor for the time being so I'll just make it work, but might be worth exploring in a future cleanup. Potentially refactor to be more like aws-cdk-lib by dropping all the lib/construct
prefixes and moving all the constructs up to be submodules directly of @guardian/cdk
:
@guardian/cdk
-> Patterns
@guardian/cdk/(AWS Service)
-> Constructs for (AWS Service)
@guardian/cdk/lib/util
-> Utility functions
@guardian/cdk/lib/constants
-> Constant
@guardian/cdk/lib/riff-raff
-> Riff Raff YAML generator
@guardian/cdk/experimental/*
- All of the above prefixed with experimental
This PR is stale because it has been open 30 days with no activity. Unless a comment is added or the “stale” label removed, this will be closed in 3 days |
What does this change?
Add explicit exports to our
package.json
I always seem to have issues with Intellisense and
@guardian/cdk
, Intellisense seems to work fine with regularaws-cdk-lib
constructs but it can almost never find @guardian/cdk exports. From asking about it sounds like I'm not the only user affected by this.Furthermore Deno is able to compile and synth regular
aws-cdk
projects, but the moment we include@guardian/cdk
it starts complaining about being unable to find imports for a lot of @guardian/cdk classes.My suspicion is because
@guardian/cdk
exports each construct as its own module instead of exporting everything through the rootindex.ts
file.aws-cdk-lib
does this too but it explicitly specifies all its exports in its package.jsonHow to test
Copied the
package.json
to my node_modules folder.Before:
After:
How can we measure success?
Have we considered potential risks?
Checklist
Footnotes
Consider whether this is something that will mean changes to projects that have already been migrated, or to the CDK CLI tool. If changes are required, consider adding a checklist here and/or linking to related PRs. ↩
If you are adding a new construct or pattern, has new documentation been added? If you are amending defaults or changing behaviour, are the existing docs still valid? ↩