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

feat: add hotjar package #264

Merged
merged 1 commit into from
Jun 22, 2022
Merged

feat: add hotjar package #264

merged 1 commit into from
Jun 22, 2022

Conversation

long74100
Copy link
Contributor

@long74100 long74100 commented Jun 17, 2022

Add hotjar package so that we can move away from relying on react-use-hotjar which was causing dependency errors in our mfes. It will also give us control over hotjar functionality from now on.

Will need to go back, fix hook errors, and remove /* eslint-disable react-hooks/exhaustive-deps */ from files.

Merge checklist:

  • Evaluate how your changes will impact existing consumers (e.g., frontend-app-learner-portal-enterprise, frontend-app-admin-portal, and frontend-app-enterprise-public-catalog). Will consumers safely be able to upgrade to this change without any breaking changes?
  • Ensure your commit message follows the semantic-release conventional commit message format. If your changes include a breaking change, ensure your commit message is explicitly marked as a BREAKING CHANGE so the NPM package is released as such.
  • Once CI is passing, verify the package versions that Lerna will increment to in the Github Action CI workflow logs.
    • Note: This may be found in the "Preview Updated Versions (dry run)" step in the Github Action CI workflow logs.

Post merge:

  • Verify Lerna created a release commit (e.g., chore(release): publish) that incremented versions in relevant package.json and CHANGELOG files, and created Git tags for those versions.
  • Run the Publish from package.json Github Action workflow to publish these new package versions to NPM.
    • This may be triggered by clicking the "Run workflow" option for the master branch.
  • Verify the new package versions were published to NPM (i.e., npm view <package_name> versions --json).
    • Note: There may be a slight delay between when the workflow finished and when NPM reports the package version as being published. If it doesn't appear right away in the above command, try again in a few minutes.

@long74100 long74100 force-pushed the add-hot-jars-lib branch 4 times, most recently from 90cc131 to 78347fc Compare June 22, 2022 17:52
@codecov
Copy link

codecov bot commented Jun 22, 2022

Codecov Report

Merging #264 (38eaf6d) into master (e9ed4c8) will increase coverage by 1.38%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #264      +/-   ##
==========================================
+ Coverage   75.99%   77.37%   +1.38%     
==========================================
  Files          33       34       +1     
  Lines         604      641      +37     
  Branches      152      160       +8     
==========================================
+ Hits          459      496      +37     
  Misses        132      132              
  Partials       13       13              
Impacted Files Coverage Δ
packages/catalog-search/src/CurrentRefinements.jsx 78.94% <ø> (ø)
packages/catalog-search/src/FacetListBase.jsx 71.05% <ø> (ø)
packages/catalog-search/src/SearchBox.jsx 94.64% <ø> (ø)
packages/catalog-search/src/SearchContext.jsx 100.00% <ø> (ø)
packages/catalog-search/src/SearchFilters.jsx 73.68% <ø> (ø)
packages/catalog-search/src/data/hooks.js 100.00% <ø> (ø)
packages/hotjar/src/hotjar.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9ed4c8...38eaf6d. Read the comment docs.

package.json Outdated
@@ -16,6 +16,7 @@
"clean": "npm run clean --workspaces && rm -rf ./node_modules",
"build": "npm run build --workspaces",
"test": "npm run test --workspaces",
"test:watch": "npm run test -- --watch",
Copy link
Member

Choose a reason for hiding this comment

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

Should this command be npm run test:watch --workspaces to run test:watch in each package in the workspace?

.. |Build Status| image:: https://github.com/edx/frontend-enterprise/actions/workflows/release.yml/badge.svg
:target: https://github.com/edx/frontend-enterprise/actions
.. |npm_version| image:: https://img.shields.io/npm/v/@edx/frontend-enterprise-utils.svg
:target: @edx/frontend-enterprise-utils
Copy link
Member

Choose a reason for hiding this comment

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

nit: @edx/frontend-enterprise-utils -> @edx/frontend-enterprise-hotjar throughout this file

@@ -0,0 +1,88 @@
/* eslint-disable no-console */

Copy link
Member

Choose a reason for hiding this comment

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

It might be a good idea to drop a link to Github where we sourced/adapted this from for future reference.

Copy link
Member

@adamstankiewicz adamstankiewicz left a comment

Choose a reason for hiding this comment

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

Premature approval once any nits/suggestions are addressed 😄

@long74100 long74100 merged commit f132ce1 into master Jun 22, 2022
@long74100 long74100 deleted the add-hot-jars-lib branch June 22, 2022 20:43
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

2 participants