Skip to content
This repository was archived by the owner on Sep 3, 2022. It is now read-only.

Replace @ndhoule/defaults with ES6 spread syntax merging #185

Merged
merged 7 commits into from
Sep 17, 2020

Conversation

bryanmikaelian
Copy link
Contributor

@bryanmikaelian bryanmikaelian commented Aug 19, 2020

Description

This PR rips out the third party library @ndhoule/defaults and replaces it with merges via the ES6's object spread syntax.

Test plan

Testing completed successfully using verifying existing unit tests pass and adding additional test coverage. Also verified with e2e tests using the scenarios below:

Each scenario shows me overriding context.page.search with lol

  • properties merged with pageDefaults on a Track call and supports overrides
    Screen Shot 2020-09-16 at 5 52 46 PM

  • properties merged with pageDefaults on a Page call and supports overrides
    Screen Shot 2020-09-16 at 5 54 53 PM

Note: There is an existing bug where the properties doesn't reflect changes to context.page. This does not happen in other methods

  • context.page with defaults exists on a Identify call and supports overrides
    Screen Shot 2020-09-16 at 5 53 57 PM

  • context.page with defaults exists on an Alias call and supports overrides
    Screen Shot 2020-09-16 at 5 55 20 PM

  • cookie defaults are set
    Screen Shot 2020-09-17 at 8 33 36 AM

  • store enabled by default and values are stored (tested with Identify)
    Screen Shot 2020-09-17 at 8 38 55 AM

Checklist

  • Thorough explanation of the issue/solution, and a link to the related issue
  • CI tests are passing
  • Unit tests were written for any new code
  • Code coverage is at least maintained, or increased.

@bryanmikaelian bryanmikaelian changed the title Replace @ndhoule/defaults with Object.assign and ES6 merging Replace @ndhoule/defaults with Object.assign and ES6 spread syntax merging Aug 19, 2020
@bryanmikaelian bryanmikaelian force-pushed the bryan/replace-with-native-functions branch 2 times, most recently from cd461e3 to aab4d19 Compare August 20, 2020 15:40
@bryanmikaelian bryanmikaelian changed the title Replace @ndhoule/defaults with Object.assign and ES6 spread syntax merging Replace @ndhoule/defaults with lodash.assign and ES6 spread syntax merging Aug 20, 2020
@bryanmikaelian bryanmikaelian marked this pull request as ready for review August 20, 2020 15:50
@bryanmikaelian bryanmikaelian force-pushed the bryan/replace-with-native-functions branch 3 times, most recently from f679c89 to c1247d7 Compare August 24, 2020 16:55
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@ed0a5fa). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #185   +/-   ##
=========================================
  Coverage          ?   96.36%           
=========================================
  Files             ?        3           
  Lines             ?       55           
  Branches          ?        0           
=========================================
  Hits              ?       53           
  Misses            ?        2           
  Partials          ?        0           
Impacted Files Coverage Δ
lib/utils/each.js 90.47% <0.00%> (ø)
lib/utils/map.js 100.00% <0.00%> (ø)
lib/utils/clone.js 100.00% <0.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 ed0a5fa...c1247d7. Read the comment docs.

@bryanmikaelian bryanmikaelian force-pushed the bryan/replace-with-native-functions branch from c1247d7 to 9c0f101 Compare September 16, 2020 20:51
@bryanmikaelian bryanmikaelian changed the title Replace @ndhoule/defaults with lodash.assign and ES6 spread syntax merging Replace @ndhoule/defaults with ES6 spread syntax merging Sep 16, 2020
@bryanmikaelian bryanmikaelian force-pushed the bryan/replace-with-native-functions branch from 7d8faaf to de834bf Compare September 17, 2020 12:25
@bryanmikaelian bryanmikaelian marked this pull request as ready for review September 17, 2020 12:56
@bryanmikaelian bryanmikaelian requested a review from a team September 17, 2020 12:56
@bryanmikaelian bryanmikaelian merged commit 7b59f34 into master Sep 17, 2020
@bryanmikaelian bryanmikaelian deleted the bryan/replace-with-native-functions branch September 17, 2020 20:15
juliofarah added a commit that referenced this pull request Mar 19, 2021
hbrls pushed a commit to nice-fungal/analytics.js-core that referenced this pull request Apr 16, 2021
hbrls pushed a commit to nice-fungal/analytics.js-core that referenced this pull request Apr 16, 2021
hbrls pushed a commit to nice-fungal/analytics.js-core that referenced this pull request Apr 16, 2021
hbrls pushed a commit to nice-fungal/analytics.js-core that referenced this pull request Apr 16, 2021
hbrls pushed a commit to nice-fungal/analytics.js-core that referenced this pull request May 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants