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

fix: remove /src/ from .npmignore (for sourcemaps) #393

Closed
wants to merge 4 commits into from

Conversation

justingrant
Copy link
Contributor

@justingrant justingrant commented Oct 10, 2019

Crrently, SDKV3 publishes sourcemaps to npm but doesn't publish the source files that those sourcemaps point to. This missing source files breaks debugging use-cases, especially for VSCode users because the VSCode debugger relies on source files for setting breakpoints in the debugger, for debugger call stacks, for "step into" original source, and any other debugger use-cases. Even outside of debugging use-cases, it's helpful for developers consuming transpiled libraries to have original source so they can better understand what the library is doing. Finally, some tools will show warnings when sourcemaps are present but source files are missing, and this PR will fix these warnings.

This PR:

  • removes /src from npm for non-client packages, but adds adds jest.config.js and *.spec.ts to these packages' .npmignore because it's not expected to be able to run your tests using an NPM download.
  • removes *.ts from npm for client packages. (also removes !*.d.ts which is now unnecessary)
  • in 10 non-client packages, changes sourceRoot to rootDir in tsconfig.json. The latter setting is used by all other SDKV3 packages, and it produces sourcemaps with an empty sourceRoot field. This is ideal, because the sourceRoot feature turned out to be buggy in some sourcemap-reading and sourcemap-writing tools. Leaving it blank maximizes compatibility with more tools.

This PR is similar to aws/aws-sdk-js-crypto-helpers#5 in the AWS Crypto Helpers library, and aws-amplify/amplify-js#2680 which made similar changes to AWS Amplify library earlier this year.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@aws-sdk-js-automation

This comment has been minimized.

@aws-sdk-js-automation
Copy link

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@justingrant
Copy link
Contributor Author

After my initial commit last week, I made a few additional changes:

  • omitted test-only files from npm
  • fixed a sourcemap-configuration issue with 10 non-client projects.

I updated the initial comment to reflect these additional changes.

@trivikr trivikr force-pushed the master branch 2 times, most recently from f2fdf2e to 4a23bb4 Compare January 2, 2020 21:54
@trivikr
Copy link
Member

trivikr commented Jan 28, 2020

Thank you @justingrant for posting this PR.
Closing as we no longer exclude TypeScript files in alpha clients

The code is currently in smithy-codegen branch, and would be merged to master when it's stable.

/coverage/
/docs/
tsconfig.test.json
*.tsbuildinfo

Verified as follows:

$ yarn add @aws-sdk/client-acm@alpha
$ yarn list @aws-sdk/client-acm
└─ @aws-sdk/client-acm@1.0.0-alpha.12
$ ls node_modules/@aws-sdk/client-acm 
ACM.ts                          node_modules
ACMClient.ts                    package.json
CHANGELOG.md                    protocols
LICENSE                         runtimeConfig.browser.ts
README.md                       runtimeConfig.shared.ts
commands                        runtimeConfig.ts
dist                            tsconfig.es.json
endpoints.ts                    tsconfig.json
index.ts                        types
models

@trivikr trivikr closed this Jan 28, 2020
@lock
Copy link

lock bot commented Feb 4, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked as resolved and limited conversation to collaborators Feb 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants