Skip to content

Conversation

@kevinpagtakhan
Copy link
Contributor

@kevinpagtakhan kevinpagtakhan commented Dec 2, 2021

Summary

This change includes a fix to the build step that generates amplitude-segment-snippet.min.js.

Currently, the build steps fails on minify, after grep pipes code to minify that contains syntax errors. The syntax error stems from to an incorrect pattern used in grep. The pattern was previously working with unstyled code until prettier was introduced in this commit.

After prettier was introduced, the following line in src/amplitude-snippet.js:

as.onload = function() {if(!window.amplitude.runQueuedFunctions) {console.log('[Amplitude] Error: could not load SDK');}};

turned into:

as.onload = function () {
  if (!window.amplitude.runQueuedFunctions) {
    console.log('[Amplitude] Error: could not load SDK');
  }
};

where the pattern does not work with. This leads to code containing syntax errors getting piped to minify and ultimately causing it to fail. The output is an empty file named amplitude-segment-snippet.min.js.

In addition, with the output file being empty, it breaks the release workflow for @semantic-release/github in the step where it publishes the file that matches this blob. This publish step validates that the file is not empty. The emptyamplitude-segment-snippet.min.js violates the rule leading to a failed release workflow.

The change assumes that amplitude-segment-snippet.min.js is still relevant and needs to be kept. If the assumption is not true, we can alternatively remove the build step for this file.

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?: No

@kevinpagtakhan kevinpagtakhan self-assigned this Dec 2, 2021
@kevinpagtakhan kevinpagtakhan marked this pull request as ready for review December 2, 2021 04:30
Copy link
Contributor

@jooohhn jooohhn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the well researched write up

@kevinpagtakhan kevinpagtakhan merged commit 090ce31 into main Dec 2, 2021
@kevinpagtakhan kevinpagtakhan deleted the AMP-36354-fix-build branch December 2, 2021 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants