-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 file mapping in dev #9475
Fix file mapping in dev #9475
Conversation
Hey, I noticed you changed some webpack configuration files. This can break production builds. Did you remember to run a production build locally to verify they still work? |
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.
We should add No QA
to the QA testing steps.
Just for the sake of completion, can you please verify that adding |
Just tested it and it works. I added it to the test steps too. |
Thanks! |
Merging this since it’s a simple change and fixes debugging. |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by @luacmartins in version: 1.1.78-2 🚀
|
🚀 Deployed to production by @sketchydroide in version: 1.1.78-8 🚀
|
The mapping from the bundle JS to the original JS is broken in dev. You can reproduce this by doing a simple
console.log
and clicking the link that takes your to the line of code where it came from. It is taking to the incorrect line.For example: I put a
console.log
in createOnyxContext.js, but the console logs shows:Details
This is implementing the solution proposed by @kidroca here, which worked for me
Fixed Issues
https://expensify.slack.com/archives/C01GTK53T8Q/p1655212769598969
Tests
console.log
somewhere and make sure that the line is executedQA
No QA required
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)PR Reviewer Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesSTYLE.md
) were followed/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)QA Steps
Screenshots
Web
Mobile Web
Desktop
iOS
Android
cc @tgolen @luacmartins