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.

Sign up for GitHub

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

Jump to bottom

Ensure that marshalArgs pulls wildcard info value from __data #5476

Merged
merged 8 commits into from
Feb 4, 2019
Merged

Ensure that marshalArgs pulls wildcard info value from __data #5476

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4d99099
Ensure that marshalArgs pulls wildcard info value from __data
kevinpschaaf Jan 31, 2019
c78f679
Refactor to make code more readable, add tests, remove dead code.
kevinpschaaf Feb 1, 2019
2480b25
Upgrade webcomponentsjs
kevinpschaaf Feb 1, 2019
98304fb
Updates based on review.
kevinpschaaf Feb 4, 2019
09fa985
Upgrade wcjs
kevinpschaaf Feb 4, 2019
0c85340
Fix lint errors.
kevinpschaaf Feb 4, 2019
d3f27d0
Add issue for TODO
kevinpschaaf Feb 4, 2019
67caf45
Add comment re: undefined issue
kevinpschaaf Feb 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 25 additions & 30 deletions lib/mixins/property-effects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -964,6 +964,19 @@ function parseArg(rawArg) {
return a;
}

function getArgValue(data, props, structured, path) {
if (structured) {
let value = get(data, path);
// when data is not stored e.g. `splices`, get the changed value
if (value === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

So we want to pull the live value here, but we fall back to the stored value in props for splices. Adding a specific check for splices would be best but might be a performance hit. What if we take the value in props only if it is not === undefined? This allows the live value to be undefined which it sometimes can be.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added skipped tests and TODO to address this in a separate change.

value = props[path];
}
return value;
} else {
return data[path];
}
}

// data api

/**
Expand All @@ -982,8 +995,6 @@ function notifySplices(inst, array, path, splices) {
let splicesPath = path + '.splices';
inst.notifyPath(splicesPath, { indexSplices: splices });
inst.notifyPath(path + '.length', array.length);
// Null here to allow potentially large splice records to be GC'ed.
inst.__data[splicesPath] = {indexSplices: null};
}

/**
Expand Down Expand Up @@ -2140,37 +2151,21 @@ export const PropertyEffects = dedupingMixin(superClass => {
*/
_marshalArgs(args, path, props) {
const data = this.__data;
let values = [];
const values = [];
for (let i=0, l=args.length; i<l; i++) {
let arg = args[i];
let name = arg.name;
let v;
if (arg.literal) {
v = arg.value;
} else {
if (arg.structured) {
v = get(data, name);
// when data is not stored e.g. `splices`
if (v === undefined) {
v = props[name];
}
} else {
v = data[name];
let {name, structured, wildcard, value, literal} = args[i];
if (!literal) {
value = getArgValue(data, props, structured, name);
if (wildcard) {
const matches = isDescendant(name, path);
value = {
path: matches ? path : name,
value: matches ? getArgValue(data, props, structured, path) : value,
base: value
};
}
}
if (arg.wildcard) {
// Only send the actual path changed info if the change that
// caused the observer to run matched the wildcard
let baseChanged = (name.indexOf(path + '.') === 0);
let matches = (path.indexOf(name) === 0 && !baseChanged);
values[i] = {
path: matches ? path : name,
value: matches ? props[path] : v,
base: v
};
} else {
values[i] = v;
}
values[i] = value;
}
return values;
}
Expand Down
Loading