Skip to content
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

Maintenance: type exports in Parameters & Idempotency pkgs are incorrect #2284

Closed
webdeveric opened this issue Mar 27, 2024 · 7 comments · Fixed by #2285
Closed

Maintenance: type exports in Parameters & Idempotency pkgs are incorrect #2284

webdeveric opened this issue Mar 27, 2024 · 7 comments · Fixed by #2285
Assignees
Labels
completed This item is complete and has been merged/shipped enhancement PRs that introduce minor changes, usually to existing features idempotency This item relates to the Idempotency Utility parameters This item relates to the Parameters Utility

Comments

@webdeveric
Copy link

Expected Behaviour

The exports should be valid for the Node.js runtime and point to files that exist and are the correct file type.

Current Behaviour

Some packages have slightly incorrect exports.

For example, packages/idempotency/package.json currently has an import and require pointing to a .d.ts file. This should be a .js file.

There are also issues in packages/parameters/package.json (same as above) and packages/jmespath/package.json (missing files)

Code snippet

These should be .js files.

"./types": {
"import": "./lib/esm/types/index.d.ts",
"require": "./lib/cjs/types/index.d.ts"
}

"./ssm/types": {
"import": "./lib/esm/types/SSMProvider.d.ts",
"require": "./lib/cjs/types/SSMProvider.d.ts"
},

The files referenced here do not exist.

"./envelopes": {
"import": "./lib/esm/envelopes.js",
"require": "./lib/cjs/envelopes.js"
},

Steps to Reproduce

I recently released https://www.npmjs.com/package/validate-package-exports to help validate packages I publish to make sure I don't miss anything. Since I use Powertools, I thought I'd use it as a demo package while developing the tool.

To validate all packages, run this in the root of the repo.

find ./packages/ -maxdepth 2 -name package.json -exec npx validate-package-exports -p {} -vs \;

If you want to validate one at a time, use this:

npx validate-package-exports  --check --verify --package [PATH-TO-PACKAGE.JSON]

Possible Solution

Update */types exports to look like this.

"./types": {
  "require": {
    "types": "./lib/cjs/types/index.d.ts",
    "default": "./lib/cjs/types/index.js"
  },
  "import": {
    "types": "./lib/esm/types/index.d.ts",
    "default": "./lib/esm/types/index.js"
  }
}

Powertools for AWS Lambda (TypeScript) version

latest

AWS Lambda function runtime

20.x

Packaging format used

npm

Execution logs

No response

@webdeveric webdeveric added triage This item has not been triaged by a maintainer, please wait bug Something isn't working labels Mar 27, 2024
@dreamorosi dreamorosi self-assigned this Mar 27, 2024
@dreamorosi
Copy link
Contributor

Hi @webdeveric, thank you for taking the time to test against our packages and report the issue - I really appreciate it!

Of the three packages mentioned, only Idempotency and Parameters have valid issues - the third one is still under development and the file that was missing was merged a couple hrs ago in #2266.

I am a bit on the fence to classify this as a bug, since according to our definition of bug this would not necessarily be one given that the exports that had issues were only for types that are consumed at dev time and never used at runtime.

Regardless of the semantics, we should fix this and make it available in the next release.

I’ll work on a PR shortly.

@dreamorosi dreamorosi moved this from Triage to Working on it in Powertools for AWS Lambda (TypeScript) Mar 27, 2024
@dreamorosi dreamorosi added idempotency This item relates to the Idempotency Utility parameters This item relates to the Parameters Utility confirmed The scope is clear, ready for implementation enhancement PRs that introduce minor changes, usually to existing features and removed bug Something isn't working triage This item has not been triaged by a maintainer, please wait labels Mar 27, 2024
@dreamorosi dreamorosi changed the title Bug: Incorrect package.json exports Maintenance: type exports in Parameters & Idempotency pkgs are incorrect Mar 27, 2024
@dreamorosi dreamorosi linked a pull request Mar 27, 2024 that will close this issue
9 tasks
@github-project-automation github-project-automation bot moved this from Working on it to Coming soon in Powertools for AWS Lambda (TypeScript) Mar 27, 2024
Copy link
Contributor

⚠️ COMMENT VISIBILITY WARNING ⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

@github-actions github-actions bot added pending-release This item has been merged and will be released soon and removed confirmed The scope is clear, ready for implementation labels Mar 27, 2024
@dreamorosi
Copy link
Contributor

As a side note, I was wondering if you’d be open to feature requests for your validate-package-exports package.

I’d like to explore the possibility of adding your validate-package-exports package to our CI to catch these types of issues earlier.

Before doing so however, I think we’d need some small tweaks like having the ability to customize the output further, specifically for misconfigured exports.

Right now the output is fairly verbose and contains the entire stack trace. Ideally there could be a way to output a JSON or another parseable format like I suggested in this feature request webdeveric/validate-package-exports#1.

The main reason for that request is that at least for our workflow, this type of package seems to be a great candidate for a GitHub Action rather than being installed as a package in the repo and ran as a CI.

In that way any misconfiguration would be surfaced in a PR as a failed check, so that PR authors can fix the issue immediately. Having the outputs as they are now would require some extra steps to open the CI logs, comb through them, and then finally find the correct line.

@webdeveric
Copy link
Author

@dreamorosi My personal preference is to run it in a postbuild script or pre-commit hook so broken configurations never make it into GitHub. I would be open to making a GH Action for this if it provides more useful information to a PR.

@dreamorosi
Copy link
Contributor

Yes, I understand.

For us we try to strike a balance between commit hooks and CI checks. We try not to add too many checks locally because we found that when we do that devs tend to just commit using —no-verify and skip them as they feel like it slow down things.

But ultimately it’s a topic that comes down to personal/team preference and we tweak things continuously.

For this specific case, I don’t foresee us modifying the exports too often so having a check in the PR so that the issue doesn’t make it to main is valuable enough and keeps the feedback loop on local quick enough.

And just to be clear, I don’t expect you to create the GH Action - if we could have the JSON output we’ll likely create one for our project and customize the output to our needs anyway.

@dreamorosi
Copy link
Contributor

Thanks to the quick changes in the CLI over the past few days I was able to create a GitHub Action that runs the CLI and displays any error using GitHub's annotations.

on-pr-update

You can find the action here: https://github.com/dreamorosi/validate-package-exports-action - your package is credited at the very beginning of the readme.

Over the next few days I will improve test coverage and do more manual tests. Once that's done I will cut a v1 release and add it to this repo.

Copy link
Contributor

This is now released under v2.0.4 version!

@github-actions github-actions bot added completed This item is complete and has been merged/shipped and removed pending-release This item has been merged and will be released soon labels Apr 10, 2024
@dreamorosi dreamorosi moved this from Coming soon to Shipped in Powertools for AWS Lambda (TypeScript) Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed This item is complete and has been merged/shipped enhancement PRs that introduce minor changes, usually to existing features idempotency This item relates to the Idempotency Utility parameters This item relates to the Parameters Utility
Projects
2 participants