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: augments metadata gathering for @wire decorator #631

Merged
14 commits merged into from Sep 14, 2018
@@ -1,5 +1,39 @@
const pluginTest = require('./utils/test-transform').pluginTest(require('../index'));

const wireMetadataParameterTest =
(testName, {declaration = '', wireParameters = '', expectedStaticParameters = {}, expectedVariableParameters = {}}) => {
pluginTest(
testName,
`
import { wire } from 'lwc';
import { getRecord } from 'recordDataService';
${declaration};
export default class Test {
@wire(getRecord, { ${wireParameters.join(',')} })
recordData;
}
`,
{
output: {
metadata: {
decorators: [{
type: 'wire',
targets: [
{
adapter: { name: 'getRecord', reference: 'recordDataService' },
name: 'recordData',
params: expectedVariableParameters,
static: expectedStaticParameters ,
type: 'property',
}
],
}]
},
},
},
);
};

describe('Transform property', () => {
pluginTest('transforms wired field', `
import { wire } from 'lwc';
Expand Down Expand Up @@ -126,7 +160,7 @@ Test.wire = {
}
};`
}
})
});

pluginTest('decorator accepts an optional config object as second parameter', `
import { wire } from 'lwc';
Expand Down Expand Up @@ -331,7 +365,7 @@ Test.wire = {

describe('Metadata', () => {
pluginTest(
'gather track metadata',
'gather wire metadata',
`
import { wire } from 'lwc';
import { getFoo } from 'data-service';
Expand All @@ -352,19 +386,102 @@ describe('Metadata', () => {
adapter: { name: 'getFoo', reference: 'data-service' },
name: 'wiredProp',
params: { key1: 'prop1' },
static: { key2: ['fixed'] },
static: { key2: { value: ['fixed'], type: 'array' } },
type: 'property',
},
{
adapter: { name: 'getFoo', reference: 'data-service' },
name: 'wiredMethod',
params: { key1: 'prop1' },
static: { key2: ['fixed'] },
static: { key2: { value: ['fixed'], type: 'array' } },
type: 'method',
}],
}]
},
},
},
);

wireMetadataParameterTest('when constant initialised to a string-literal should extract the value',
{ declaration: `const stringConstant = '123';`,
wireParameters: ['userId: stringConstant'],
expectedStaticParameters: { userId: { value: '123', type: 'string'} } });

wireMetadataParameterTest('when constant initialised to a number-literal should extract the value',
{ declaration: `const numberConstant = 100;`,
wireParameters: ['size: numberConstant'],
expectedStaticParameters: { size: { value: 100, type: 'number'} } });

wireMetadataParameterTest('when constant initialised to a boolean-literal should extract the value',
{ declaration: `const booleanConstant = true;`,
wireParameters: ['isRegistered: booleanConstant'],
expectedStaticParameters: { isRegistered: { value: true, type: 'boolean'} } });

wireMetadataParameterTest('when constant initialised as a reference to another should mark as unresolved',
{ declaration: `const stringConstant = '123'; const referenceConstant = stringConstant;`,
wireParameters: ['recordId: referenceConstant'],
expectedStaticParameters: { recordId: { type: 'unresolved', value: 'reference' } } });

wireMetadataParameterTest('when importing a default export from a module should reference the name of the module',
{ declaration: `import id from '@salesforce/user/Id';`,
wireParameters: ['recordId: id'],
expectedStaticParameters: { recordId: { value: '@salesforce/user/Id', type: 'module' } } });

wireMetadataParameterTest('when parameter reference missing should mark as undefined',
{ wireParameters: ['recordId: id'],
expectedStaticParameters: { recordId: { type: 'unresolved', value: 'reference'} } });

wireMetadataParameterTest('when importing named export with "as" from a module should reference the name of the module',
{ declaration: `import { id as currentUserId } from '@salesforce/user/Id';`,
wireParameters: ['recordId: currentUserId'],
expectedStaticParameters: { recordId: { value: '@salesforce/user/Id', type: 'module' } } });
This conversation was marked as resolved.
Show resolved Hide resolved

wireMetadataParameterTest('when importing a named export from a module should reference the name of the module',
{ declaration: `import { id } from '@salesforce/user/Id';`,
wireParameters: ['recordId: id'],
expectedStaticParameters: { recordId: { value: '@salesforce/user/Id', type: 'module' } } });

wireMetadataParameterTest('when importing from a relative module should reference the name of the module',
{ declaration: `import id from './someReference.js';`,
wireParameters: ['recordId: id'],
expectedStaticParameters: { recordId: { value: './someReference.js', type: 'module' } } });

wireMetadataParameterTest('when referencing a "let" variable should mark as undefined',
{ declaration: `let userId = '123';`,
wireParameters: ['recordId: userId'],
expectedStaticParameters: { recordId: { type: 'unresolved', value: 'reference'} } });

wireMetadataParameterTest('when referencing a member expression, should mark as undefined with type object',
This conversation was marked as resolved.
Show resolved Hide resolved
{ declaration: `const data = {userId: '123'};`,
wireParameters: ['recordId: data.userId'],
expectedStaticParameters: { recordId: { type: 'unresolved', value: 'member_expression' } } });

wireMetadataParameterTest('when an inline string-literal initialization is used, should use value',
{ wireParameters: ['recordId: "123"'],
expectedStaticParameters: { recordId: { value: '123', type: 'string' } } });

wireMetadataParameterTest('when an inline numeric-literal initialization is used, should use value',
{ wireParameters: ['size: 100'],
expectedStaticParameters: { size: { value: 100, type: 'number' } } });

wireMetadataParameterTest('when an inline float-literal initialization is used, should use value',
{ wireParameters: ['underPrice: 50.50'],
expectedStaticParameters: { underPrice: { value: 50.50, type: 'number' } } });

wireMetadataParameterTest('when an inline boolean-literal initialization is used, should use value',
{ wireParameters: ['isRegistered: true'],
expectedStaticParameters: { isRegistered: { value: true, type: 'boolean' } } });

wireMetadataParameterTest('when $foo parameters are used, should use name of the parameter',
{ wireParameters: ['recordId: "$userId"'],
expectedVariableParameters: { recordId: 'userId' } });

wireMetadataParameterTest('when $foo parameters with dots are used, should use name of the parameter',
This conversation was marked as resolved.
Show resolved Hide resolved
{ wireParameters: ['recordId: "$userRecord.Id"'],
expectedVariableParameters: { recordId: 'userRecord.Id' } });

wireMetadataParameterTest('when inline array with a non-string literal is used, should have "undefined" for non-string literal values',
{ declaration: `const bar = '123';`,
wireParameters: ['fields: ["foo", bar]'],
expectedStaticParameters: { fields: {type: 'array', value: ['foo', undefined]} } });
Copy link
Contributor

Choose a reason for hiding this comment

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

i wonder if it's good to do this or treat the whole thing as unresolvable

Copy link
Author

Choose a reason for hiding this comment

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

I was pondering about that, it seems somewhat useful as you'd at least be able to analyze metadata and then figure out that say due to just one parameter an expensive query wasn't pre-cached so action may be taken ?

Up to you though, if you feel that the whole thing should be undefined I can change that.

});
Expand Up @@ -74,12 +74,49 @@ function buildWireConfigValue(t, wiredValues) {
}));
}

function getWiredStaticMetadata(properties) {
const ret = {};
const SUPPORTED_VALUE_TYPE_TO_METADATA_TYPE = {
StringLiteral: 'string',
NumericLiteral: 'number',
BooleanLiteral: 'boolean'
};

function getWiredStaticMetadata(properties, referenceLookup) {
const ret = {};
properties.forEach(s => {
if (s.key.type === 'Identifier' && s.value.type === 'ArrayExpression') {
ret[s.key.name] = s.value.elements.map(e => e.value);
let result = {};
const valueType = s.value.type;
if (s.key.type === 'Identifier') {
if (valueType === 'ArrayExpression') {
// @wire(getRecord, { fields: ['Id', 'Name'] })
result = {type: 'array', value: s.value.elements.map(e => e.value)};
Copy link
Member

Choose a reason for hiding this comment

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

Do we make sure somewhere else the code that array literal items only contains string literals? If it's the case let's add a comment otherwise, we need to be more defensive when accessing the value property.

Copy link
Author

Choose a reason for hiding this comment

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

see my comment regarding these on the appropriate test, we don't do any extra checks, the references get resolved as "undefined", up to you guys whether you want it to work like this or you want to undefine the whole thing, there are pros and cons with each approach.

} else if (SUPPORTED_VALUE_TYPE_TO_METADATA_TYPE[valueType]) {
// @wire(getRecord, { companyName: ['Acme'] })
// @wire(getRecord, { size: 100 })
// @wire(getRecord, { isAdmin: true })
result = {type: SUPPORTED_VALUE_TYPE_TO_METADATA_TYPE[valueType], value: s.value.value};
} else if (valueType === 'Identifier') {
// References such as:
// 1. Modules
// import id from '@salesforce/user/id'
// @wire(getRecord, { userId: id })
//
// 2. 1st order constant references with string literals
// const userId = '123';
// @wire(getRecord, { userId: userId })
const reference = referenceLookup(s.value.name);
result = {value: reference.value, type: reference.type};
Copy link
Member

Choose a reason for hiding this comment

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

Let's hardcode identifier in type, the reference.type is equal to Identifier (with a capital I) and will not be consistent with other types.

Copy link
Author

@ghost ghost Sep 13, 2018

Choose a reason for hiding this comment

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

not sure I understand? the reference.type has nothing to do with babel we set it ourselves in getReferenceByName, it's either going to be 'module' or 'string'/'number'/'boolean' (or undefined)

Copy link
Member

Choose a reason for hiding this comment

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

Oh my bad.

if (!result.type) {
result = {type: 'unresolved', value: 'reference'}
}
} else if (valueType === 'MemberExpression') {
// @wire(getRecord, { userId: recordData.Id })
result = {type: 'unresolved', value: 'member_expression'};
}
}
if (!result.type) {
result = {type: 'unresolved'};
}
This conversation was marked as resolved.
Show resolved Hide resolved
ret[s.key.name] = result;
});
return ret;
}
Expand All @@ -94,9 +131,39 @@ function getWiredParamMetadata(properties) {
return ret;
}

const scopedReferenceLookup = scope => name => {
const binding = scope.getBinding(name);
This conversation was marked as resolved.
Show resolved Hide resolved

let type;
let value;

if (binding) {
if (binding.kind === 'module') {
// Resolves module import to the name of the module imported
// e.g. import { foo } from 'bar' gives value 'bar' for `name == 'foo'
let parentPathNode = binding.path.parentPath.node;
if (parentPathNode && parentPathNode.source) {
type = 'module';
value = parentPathNode.source.value;
}
} else if (binding.kind === 'const') {
// Resolves `const foo = 'text';` references to value 'text', where `name == 'foo'`
const init = binding.path.node.init;
if (init && SUPPORTED_VALUE_TYPE_TO_METADATA_TYPE[init.type]) {
type = SUPPORTED_VALUE_TYPE_TO_METADATA_TYPE[init.type];
value = init.value;
}
}
}
return {
type,
value
};
};

module.exports = function transform(t, klass, decorators) {
const metadata = [];
const wiredValues = decorators.filter(isWireDecorator).map(({ path }) => {
const wiredValues = decorators.filter(isWireDecorator).map(({path}) => {
const [id, config] = path.get('arguments');

const propertyName = path.parentPath.get('key.name').node;
Expand All @@ -107,17 +174,21 @@ module.exports = function transform(t, klass, decorators) {
const wiredValue = {
propertyName,
isClassMethod
}
};

if (config) {
wiredValue.static = getWiredStatic(config);
wiredValue.params = getWiredParams(t, config);
}

const referenceLookup = scopedReferenceLookup(path.scope);

if (id.isIdentifier()) {
const adapterName = id.node.name;
const reference = referenceLookup(adapterName);
wiredValue.adapter = {
name: id.node.name,
reference: path.scope.getBinding(id.node.name).path.parentPath.node.source.value
name: adapterName,
reference: reference.type === 'module' ? reference.value : undefined
}
}

Expand All @@ -128,7 +199,7 @@ module.exports = function transform(t, klass, decorators) {
};

if (config) {
wireMetadata.static = getWiredStaticMetadata(wiredValue.static);
wireMetadata.static = getWiredStaticMetadata(wiredValue.static, referenceLookup);
wireMetadata.params = getWiredParamMetadata(wiredValue.params);
}

Expand All @@ -154,4 +225,4 @@ module.exports = function transform(t, klass, decorators) {
targets: metadata
};
}
}
};