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(wire-service): imported function identifier as adapter id for @wire #26

Merged
merged 6 commits into from
Jan 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ describe('Wired field', () => {
export default class Test {}
Test.wire = {
innerRecord: {
type: "record",
params: { recordId: "recordId" },
static: { fields: ["Account", 'Rate'] }
static: { fields: ["Account", 'Rate'] },
type: "record"
}
};
`);
Expand All @@ -31,17 +31,37 @@ describe('Wired field', () => {
},
});

test('decorator expects a string as first parameter', `
test('decorator expects a function identifier as first parameter', `
import { wire } from 'engine';
import { record } from 'data-service';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/record/getRecord/. That's the format we're using per https://salesforce.quip.com/tpa5AVyZ21kc

export default class Test {
@wire(record, {}) innerRecord;
}
`, `
import { wire } from 'engine';
import { record } from 'data-service';
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as 36

export default class Test {}
Test.wire = {
innerRecord: {
params: {},
static: {},
adapter: record
}
};
`);

test('decorator expects an imported identifier as first parameter', `
import { wire } from 'engine';
const RECORD = "record"
export default class Test {
@wire(RECORD, {}) innerRecord;
}
`, undefined, {
message: 'test.js: @wire expects a string as first parameter.',
loc: {
line: 3,
column: 10,
},
message: 'test.js: @wire expects a function identifier to be imported as first parameter.',
loc: {
line: 4,
column: 6,
},
});

test('decorator expects an oject as second parameter', `
Copy link
Contributor

Choose a reason for hiding this comment

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

Not you but we have a typo in object (missing b)

Expand Down Expand Up @@ -69,9 +89,9 @@ describe('Wired method', () => {
}
Test.wire = {
innerRecordMethod: {
type: "record",
params: { recordId: "recordId" },
static: { fields: ["Account", 'Rate'] },
type: "record",
method: 1
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ module.exports = function wireVisitor ({ types: t }) {
function buildWireConfigValue(wiredValues) {
return t.objectExpression(wiredValues.map(wiredValue => {
const wireConfig = [
t.objectProperty(
t.identifier('type'),
t.stringLiteral(wiredValue.type)
),
t.objectProperty(
t.identifier('params'),
t.objectExpression(wiredValue.params)
Expand All @@ -45,6 +41,25 @@ module.exports = function wireVisitor ({ types: t }) {
)
];

// TODO: deprecate type (string as adapter id once consumer has migrated to use imported identifier)
if (wiredValue.type) {
wireConfig.push(
t.objectProperty(
t.identifier('type'),
t.stringLiteral(wiredValue.type)
)
)
}

if (wiredValue.adapter) {
wireConfig.push(
t.objectProperty(
t.identifier('adapter'),
t.identifier(wiredValue.adapter)
)
)
}

if (wiredValue.isClassMethod) {
wireConfig.push(
t.objectProperty(
Expand Down Expand Up @@ -72,9 +87,16 @@ module.exports = function wireVisitor ({ types: t }) {
);
}

if (!id.isStringLiteral()) {
// TODO: deprecate string as adapter id once consumer has migrated to use imported identifier
if (!id.isStringLiteral() && !id.isIdentifier()) {
throw id.buildCodeFrameError(
`@wire expects a string or a function identifier as first parameter.`
);
}

if (id.isIdentifier() && !path.scope.getBinding(id.node.name).path.isImportSpecifier()) {
throw id.buildCodeFrameError(
`@wire expects a string as first parameter.`
`@wire expects a function identifier to be imported as first parameter.`
);
}

Expand All @@ -89,13 +111,21 @@ module.exports = function wireVisitor ({ types: t }) {
kind: 'method'
});

wiredValues.push({
const wiredValue = {
propertyName,
isClassMethod,
type: id.node.value,
static: getWiredStatic(config),
params: getWiredParams(config),
});
}

// TODO: deprecate type (string as adapter id once consumer has migrated to use imported identifier)
if (id.isStringLiteral()) {
wiredValue.type = id.node.value;
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps we can extract id.node reference into a local variable, there are multiple accesses in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could, but path.node is the common pattern to access a node in babel plugin

} else if (id.isIdentifier()) {
wiredValue.adapter = id.node.name;
}

wiredValues.push(wiredValue);

path.remove();
}
Expand Down
7 changes: 5 additions & 2 deletions packages/lwc-integration/scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,13 @@ const globalModules = {
'babel/helpers/createClass' : 'createClass',
'babel/helpers/defineProperty': 'defineProperty',
'babel/helpers/objectDestructuringEmpty': 'objectDestructuringEmpty',
'wire-service': 'WireService'
'wire-service': 'WireService',
'todo': 'Todo'
};

const baseInputConfig = {
external: function (id) {
if (id.includes('babel/helpers') || id.includes('engine') || id.includes('wire-service')) {
if (id.includes('babel/helpers') || id.includes('engine') || id.includes('wire-service') || id.includes('todo')) {
return true;
}
},
Expand Down Expand Up @@ -116,6 +117,7 @@ const baseOutputConfig = {
const engineModeFile = path.join(require.resolve(`lwc-engine/dist/umd/${isCompat ? 'es5': 'es2017'}/engine.js`));
const compatPath = path.join(require.resolve('proxy-compat-build/dist/umd/compat_downgrade.js'));
const wireServicePath = path.join(require.resolve(`lwc-wire-service/dist/umd/${isCompat ? 'es5': 'es2017'}/wire-service.js`));
const todoPath = path.join(require.resolve('../src/shared/todo.js'));

if (!fs.existsSync(engineModeFile)) {
throw new Error("Compat version of engine not generated in expected location: " + engineModeFile
Expand All @@ -126,6 +128,7 @@ if (!fs.existsSync(engineModeFile)) {
fs.copySync(compatPath, path.join(testSharedOutput, 'compat.js'));
fs.copySync(engineModeFile, path.join(testSharedOutput,'engine.js'));
fs.copySync(wireServicePath, path.join(testSharedOutput, 'wire-service.js'));
fs.copySync(todoPath, path.join(testSharedOutput, 'todo.js'));

// -- Build component tests -----------------------------------------------------=

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { Element } from 'engine';

import { serviceTodo } from 'todo';
export default class WiredMethod extends Element {
@api todoId;
@track state = { error: undefined, todo: undefined };

@wire('todo', { id: '$todoId' })
@wire(serviceTodo, { id: '$todoId' })
function(error, data) {
this.state = { error: error, todo: data };
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Element } from 'engine';

import { serviceTodo } from 'todo';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/serviceTodo/getTodo/

export default class WiredProp extends Element {
@api todoId;

@wire('todo', { id: '$todoId' })
@wire(serviceTodo, { id: '$todoId' })
todo;

get error() {
Expand Down
79 changes: 3 additions & 76 deletions packages/lwc-integration/src/shared/templates.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,88 +9,14 @@ exports.app = function (cmpName) {

exports.todoApp = function (cmpName) {
return `
import { serviceTodo } from 'todo';
import registerWireService from 'wire-service';
import { createElement, register } from 'engine';
import Cmp from '${cmpName}';

function getSubject(initialValue, initialError) {
let observer;

function next(value) {
observer.next(value);
}

function error(err) {
observer.error(err);
}

function complete() {
observer.complete();
}

const observable = {
subscribe: (obs) => {
observer = obs;
if (initialValue) {
next(initialValue);
}
if (initialError) {
error(initialError);
}
return {
unsubscribe: () => { }
};
}
};

return {
next,
error,
complete,
observable
};
}

function generateTodo(id, completed) {
return {
id,
title: 'task ' + id,
completed
};
}

const TODO = [
generateTodo(0, true),
generateTodo(1, false),
// intentionally skip 2
generateTodo(3, true),
generateTodo(4, true),
// intentionally skip 5
generateTodo(6, false),
generateTodo(7, false)
].reduce((acc, value) => {
acc[value.id] = value;
return acc;
}, {});


function serviceTodo(config) {
if (!('id' in config)) {
return undefined;
}

const todo = TODO[config.id];
if (!todo) {
const subject = getSubject(undefined, { message: 'Todo not found' });
return subject.observable;
}

return getSubject(todo).observable;
}

registerWireService(register, () => {
return {
'todo': serviceTodo
serviceTodo
};
});

Expand Down Expand Up @@ -132,6 +58,7 @@ exports.wireServiceHtml = function (cmpName, isCompat) {
<body>
${isCompat ? COMPAT : ''}
<script src="/shared/engine.js"></script>
<script src="/shared/todo.js"></script>
<script src="/shared/wire-service.js"></script>
<script src="./${cmpName}.js"></script>
</body>
Expand Down
85 changes: 85 additions & 0 deletions packages/lwc-integration/src/shared/todo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) :
typeof define === 'function' && define.amd ? define(['exports'], factory) :
(factory((global.Todo = {})));
}(this, (function (exports) {
'use strict';

function getSubject(initialValue, initialError) {
var observer;

function next(value) {
observer.next(value);
}

function error(err) {
observer.error(err);
}

function complete() {
observer.complete();
}

var observable = {
subscribe: function(obs) {
observer = obs;
if (initialValue) {
next(initialValue);
}
if (initialError) {
error(initialError);
}
return {
unsubscribe: function() {}
};
}
};

return {
Copy link
Collaborator

Choose a reason for hiding this comment

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

there will be an error if someone calls next / error/ or complete before calling observable(); Your observer is undefined until subscribe is invoked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is not exposed, the only thing that the module exposes is serviceTodo

next: next,
error: error,
complete: complete,
observable: observable
};
}

function generateTodo(id, completed) {
return {
id: id,
title: 'task ' + id,
completed: completed
};
}

var TODO = [
generateTodo(0, true),
generateTodo(1, false),
// intentionally skip 2
generateTodo(3, true),
generateTodo(4, true),
// intentionally skip 5
generateTodo(6, false),
generateTodo(7, false)
].reduce(function(acc, value) {
acc[value.id] = value;
return acc;
}, {});


function serviceTodo(config) {
if (!('id' in config)) {
return undefined;
}

var todo = TODO[config.id];
if (!todo) {
var subject = getSubject(undefined, { message: 'Todo not found' });
return subject.observable;
}

return getSubject(todo).observable;
}

exports.serviceTodo = serviceTodo;
Object.defineProperty(exports, '__esModule', { value: true });
})));
Loading