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

Change clone function to make deep copies #1910

Merged
merged 2 commits into from
Dec 4, 2017
Merged

Conversation

matthewlane
Copy link
Collaborator

Type of change

  • Bugfix

Description of change

Changes utils.cloneJson to perform a deep copy. This resolves situations where functions on objects were not being copied by the utility. The utility function was also renamed to reflect the fact that it now does a full clone rather than parsing by JSON, which drops functions.

Other information

Uses just-clone library, which adds ~176 bytes to the production build

Fixes #1869
Fixes #1878

src/utils.js Outdated
@@ -643,8 +644,8 @@ export function isSrcdocSupported(doc) {
'srcdoc' in doc.defaultView.frameElement && !/firefox/i.test(navigator.userAgent);
}

export function cloneJson(obj) {
return JSON.parse(JSON.stringify(obj));
export function clone(obj) {
Copy link
Member

Choose a reason for hiding this comment

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

should we call this deepClone?

@mkendall07 mkendall07 added the LGTM label Dec 4, 2017
@@ -107,6 +107,7 @@
"dependencies": {
"babel-plugin-transform-object-assign": "^6.22.0",
"core-js": "^2.4.1",
"gulp-sourcemaps": "^2.6.0"
"gulp-sourcemaps": "^2.6.0",
"just-clone": "^1.0.2"
Copy link
Member

Choose a reason for hiding this comment

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

nice find!

@jaiminpanchal27 jaiminpanchal27 merged commit 9e89e1f into master Dec 4, 2017
@jaiminpanchal27 jaiminpanchal27 deleted the bugfix/clone-deep branch December 4, 2017 21:04
@matthewlane matthewlane mentioned this pull request Dec 5, 2017
8 tasks
Millerrok pushed a commit to Vertamedia/Prebid.js that referenced this pull request Dec 8, 2017
* 'master' of https://github.com/prebid/Prebid.js: (43 commits)
  Merge Prebid 1.0 to Master (prebid#1936)
  Prebid.js 0.34.1 release
  Vertamedia adapter outstream support (prebid#1860)
  Expose native image-type asset dimensions on bid response object (prebid#1919)
  Remove for of (prebid#1932)
  Unit-test fix (prebid#1927)
  Remove description_url (prebid#1922)
  Trion Interactive Adapter Bugfix (prebid#1925)
  Remove config setting from Optimatic adapter (prebid#1909)
  IE bug fix (prebid#1930)
  Clarify ad unit media filtering warning (prebid#1903)
  Add ReadPeak Bid Adapter (prebid#1838)
  Change clone function to make deep copies (prebid#1910)
  Serverbid 1.0 (prebid#1802)
  sekindoUM for prebid1.0 (prebid#1777)
  update auctionId to be requestId (prebid#1896)
  bug fixed to populate userSync default values (prebid#1897)
  Increment pre version
  AdkernelAdn analytics adapter (prebid#1868)
  Justpremium Adapter: use `filter` instead of `...new Set` (prebid#1895)
  ...
vzhukovsky added a commit to aol/Prebid.js that referenced this pull request Dec 28, 2017
….34.0 to aolgithub-master

* commit 'f0ba90afa8b52de7a646d43928b2d51ee42e74a1': (53 commits)
  Added changelog entry.
  Added partners ids.
  Added dynamic ttl property for One Display and One Mobile.
  Prebid.js 0.34.1 release
  Vertamedia adapter outstream support (prebid#1860)
  Expose native image-type asset dimensions on bid response object (prebid#1919)
  Remove for of (prebid#1932)
  Unit-test fix (prebid#1927)
  Remove description_url (prebid#1922)
  Trion Interactive Adapter Bugfix (prebid#1925)
  Remove config setting from Optimatic adapter (prebid#1909)
  IE bug fix (prebid#1930)
  Clarify ad unit media filtering warning (prebid#1903)
  Add ReadPeak Bid Adapter (prebid#1838)
  Change clone function to make deep copies (prebid#1910)
  Serverbid 1.0 (prebid#1802)
  sekindoUM for prebid1.0 (prebid#1777)
  update auctionId to be requestId (prebid#1896)
  bug fixed to populate userSync default values (prebid#1897)
  Increment pre version
  ...
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* Change clone function to make deep copies

* Rename function
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.

Can't specify 'renderer' function for the bid Criteo adapter native integration problem
3 participants