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.

Sign up for GitHub

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

Jump to bottom

Include src/ files in npm release (for resolving source maps) #2680

Merged
merged 2 commits into from
Feb 20, 2019
Merged

Include src/ files in npm release (for resolving source maps) #2680

merged 2 commits into from
Feb 20, 2019

Conversation

justingrant
Copy link
Contributor

Fixes #2234

Description of changes:
Currently it's very hard to debug Amplify apps because no original source is available to the debugger because the source files are stripped out when publishing to npm. Intstead, you need to debug into the transpiled source which (especially for async methods like in the API package!) is incredibly difficult.

This PR removes src/** from .npmignore in 13 packages that previously excluded the src folder and its subdirectories.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@justingrant justingrant mentioned this pull request Feb 9, 2019
Closed
@ghost ghost assigned haverchuck Feb 20, 2019
@ghost ghost added the review label Feb 20, 2019
haverchuck
haverchuck approved these changes Feb 20, 2019
View reviewed changes
Copy link
Contributor

@haverchuck haverchuck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving and merging this into master, but will perform bundle size testing before release.

@haverchuck haverchuck merged commit b12317d into aws-amplify:master Feb 20, 2019
@ghost ghost removed the review label Feb 20, 2019
@dabit3
Copy link
Contributor

dabit3 commented Feb 20, 2019

Thanks @justingrant 💯

@haverchuck
Copy link
Contributor

Some stats on the effects of this PR (changes seem acceptable to me)

ANGULAR

Pre-src inclusion (Gzipped):

All (1.22 MB)
vendor.js (1.14 MB)
polyfills.js (44.43 KB)
styles.js (30.68 KB)
main.js (3.53 KB)
runtime.js (1.65 KB)

Post-src inclusion (Gzipped):

All (1.23 MB)
vendor.js (1.16 MB)
polyfills.js (44.43 KB)
styles.js (31.01 KB)
main.js (3.53 KB)
runtime.js (1.65 KB)

REACT

Pre-src inclusion (Gzipped)

All (419.15 KB)
static/js/2.84946141.chunk.js (416.96 KB)
static/js/main.1a223903.chunk.js (1.45 KB)
static/js/runtime~main.fdfcfda2.js (761 B)

Post-src inclusion (Gzipped)

All (419.55 KB)
static/js/2.86ea67ec.chunk.js (417.36 KB)
static/js/main.651b8711.chunk.js (1.45 KB)
static/js/runtime~main.fdfcfda2.js (761 B)

VUE

Pre-src inclusion (Gzipped)

All (293.7 KB)
js/chunk-vendors.0edb31f4.js (291.5 KB)
js/app.703fe03f.js (2.21 KB)

Post-src inclusion (Gzipped)

All (293.86 KB)
js/chunk-vendors.ff2f4b28.js (291.65 KB)
js/app.703fe03f.js (2.21 KB)

This was referenced Oct 10, 2019
Closed
Merged
@justingrant justingrant mentioned this pull request Oct 19, 2019
Closed
@justingrant justingrant mentioned this pull request Feb 5, 2020
Open
@justingrant justingrant mentioned this pull request Aug 25, 2020
Merged
justingrant added a commit to justingrant/amplify-js that referenced this pull request Oct 7, 2020
@justingrant
build: fix more sourcemap problems
02b1592 
This commit fixes a remaining handful of sourcemap issues remaining
after my previous sourcemap-related PRs aws-amplify#3059 and aws-amplify#2680:
* adds mapping for new folders: predictions, datastore,
  amplify-ui-angular, amplify-ui-components, amplify-ui-react,
  amplify-ui-storybook, amplify-ui-vue, api-graphql, api-rest
* removes webpack:/// prefix from oddball, non-file paths emitted by
  webpack (e.g. `webpack/universalModuleDefinition` or `fs (ignored)`).
  This prevents warnings from source-map-loader and related tools.
* The same code that fixes the "weird path" fix above also fixes a
  problem with aws-amplify#3059 where it would pre-pend webpack:/// as a
  fall-through case, even when the path was already a valid relative URL
  and didn't need that prefix. Also, according to @loganfsmyth (the most
  knowledgeable sourcemap expert I know), OSS library sourcemaps
  should always use relative URLs and should never use the
  webpack:/// prefix. Therefore, this PR removes that prefix.
  * updates the amplify-ui package's webpack config to emit sourcemaps
  * fixes paths to CSS files in the sourcemaps of amplify-ui package
  * adds sourcemaps for aws-amplify-vue package
@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@haverchuck haverchuck haverchuck approved these changes

@manueliglesias manueliglesias Awaiting requested review from manueliglesias

Assignees

@haverchuck haverchuck

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@justingrant @dabit3 @haverchuck