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: do not re-generate source unnecessarily #321

Merged
merged 2 commits into from Jun 20, 2019

Conversation

sgtcoolguy
Copy link
Contributor

JIRA: https://jira.appcelerator.org/browse/TIMOB-26876

Description
This explicitly tracks whether we modify the AST in our hyperloop visitors that look for hyperloop requires/imports. If we do not, avoid calling @babel/generate altogether and retain the existing original source code.

If we do modify, pass along the original source to generate and options to try and retain line numbers and comments (since we want to not change line numbers from what may already be source mapped, and we don't want to drop the //# sourceMappingURL= comment that points at/inlines the source map).

⚠️ Note that if minification is enabled it will break our source maps on Android, as the code there drops comments, doesn't ask to retain line numbers, and generally is unaware of source maps.

❓ I don't believe Windows does any rewriting of JS, so I don't think there's any fix here necessary for that platform...

- Avoids unncessary work (small perf improvement?)
- A later step already avoided unncessary re-write of JS file
- when we do re-generate, pass along options to retain line numbers and comments
so that we hopefully do not break source maps.
- Avoids unneccesary work of re-generating source and re-writing file
- When we do re-generate, pass along optiosn to retain lines and comments
so that we hopefully do not break existing source maps

Fixes TIMOB-26876
@sgtcoolguy
Copy link
Contributor Author

sgtcoolguy commented Jun 20, 2019

Nice side-effect here - should be a perf boost on both platforms, but probably more pronounced on iOS - since we don't re-generate source from the AST when unmodified, and on iOS we avoid rewriting the file.

@janvennemann janvennemann merged commit 6cdfbe2 into tidev:master Jun 20, 2019
@janvennemann janvennemann added this to the v4.0.3 milestone Jun 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants