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

re-enable production source maps #1502

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

maennchen
Copy link
Contributor

@maennchen
Copy link
Contributor Author

Amended compiled JS & Source Maps as discussed with @bcardarella

@bcardarella
Copy link
Contributor

@chrismccord 👍 from me on this, should be OK to merge

@chrismccord
Copy link
Member

@maennchen thank you! Can you back out the precompiled changes and I will rebuild after merging? We prefer to avoid precompiled builds in PRs for conflicts and security reasons. Thanks!

@bcardarella
Copy link
Contributor

@chrismccord the pre-compile is necessary to reference the sourcemap file, otherwise the js console won't know how to associate the two

@chrismccord
Copy link
Member

to be clear, I will build the the source/maps after I merge and everything else is perfect, we just prefer to avoid merging minified/compiled js since it can be an easy security backdoor as it is not human readable.

@bcardarella
Copy link
Contributor

@chrismccord ah ok, makes sense 👍

@maennchen
Copy link
Contributor Author

@chrismccord / @bcardarella The dist files are now removed.

@maennchen
Copy link
Contributor Author

@chrismccord / @bcardarella Is there any change needed on this PR?

@bcardarella
Copy link
Contributor

Should be OK to merge

@chrismccord
Copy link
Member

❤️❤️❤️🐥🔥

@maennchen maennchen deleted the sourcemaps branch July 13, 2021 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants