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

🚀 Flatten Literals in TemplateLiterals #27516

Merged

Conversation

kristoferbaxter
Copy link
Contributor

@kristoferbaxter kristoferbaxter commented Apr 1, 2020

Remove unnecessary work.

This change will eventually be upstreamed into Closure Compiler, and can be removed after upgrading to a newer version.

@kristoferbaxter kristoferbaxter added this to In progress in Performance Improvements via automation Apr 1, 2020
@kristoferbaxter kristoferbaxter changed the title 🚀Flatten Literals in TemplateLiterals 🚀 Flatten Literals in TemplateLiterals Apr 1, 2020
@kristoferbaxter kristoferbaxter marked this pull request as ready for review April 2, 2020 23:17
@kristoferbaxter kristoferbaxter merged commit 00a7285 into ampproject:master Apr 3, 2020
Performance Improvements automation moved this from In progress to Done Apr 3, 2020
* See the License for the specific language governing permissions and
* limitations under the License.
*/
let add = `/rtv/bar`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Terser seems to accomplish all of these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at the output of our build process, it is not for us. Would prefer to have less reliance on terser for these kinds of changes.

name: 'flatten-stringish-literals',
visitor: {
BinaryExpression: {
exit(path) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely. Good idea, will address in a followup.

ChrisAntaki pushed a commit to ChrisAntaki/amphtml that referenced this pull request Apr 3, 2020
* Refactor to support more cases
* Fix bug in function-declarations prohibiting other plugins from working
* yarn lock was out of date somehow
* Merge stringish literals
* Add in output for tests
* Remove console log debugging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants