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

Add support for types #26

Merged
merged 4 commits into from
May 15, 2019
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
12 changes: 8 additions & 4 deletions can-define-class.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function getKeyValue(key) {
}

function define(Base = Object) {
class Definable extends Base {
class DefineClass extends Base {
static _initDefines() {
if(!this[hasBeenDefinedSymbol]) {
let prototypeObject = this.prototype;
Expand Down Expand Up @@ -166,6 +166,10 @@ function define(Base = Object) {
}
}

static [Symbol.for("can.new")](...args) {
return new this(...args);
}

get [Symbol.for("can.isMapLike")]() {
return true;
}
Expand Down Expand Up @@ -197,11 +201,11 @@ function define(Base = Object) {
}
}

addDefinedProps.makeDefineInstanceKey(Definable);
addDefinedProps.makeDefineInstanceKey(DefineClass);

addTypeEvents(Definable);
addTypeEvents(DefineClass);

return Definable;
return DefineClass;
}

exports = module.exports = ns.DefineClass = define();
Expand Down
28 changes: 24 additions & 4 deletions define.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var canLogDev = require("can-log/dev/dev");

var stringToAny = require("can-string-to-any");
var defineLazyValue = require("can-define-lazy-value");
var dataTypes = require("can-data-types");

var MaybeBoolean = require("can-data-types/maybe-boolean/maybe-boolean"),
MaybeDate = require("can-data-types/maybe-date/maybe-date"),
Expand Down Expand Up @@ -124,16 +125,22 @@ module.exports = define = function(typePrototype, defines, baseDefine) {
var prop,
dataInitializers = Object.create(baseDefine ? baseDefine.dataInitializers : null),
// computed property definitions on _computed
computedInitializers = Object.create(baseDefine ? baseDefine.computedInitializers : null);
computedInitializers = Object.create(baseDefine ? baseDefine.computedInitializers : null),
required = new Set();

var result = getDefinitionsAndMethods(defines, baseDefine, typePrototype);
result.dataInitializers = dataInitializers;
result.computedInitializers = computedInitializers;

result.required = required;

// Goes through each property definition and creates
// a `getter` and `setter` function for `Object.defineProperty`.
canReflect.eachKey(result.definitions, function(definition, property){
// Add this as a required property
if(definition.required === true) {
required.add(property);
}

define.property(typePrototype, property, definition, dataInitializers, computedInitializers, result.defaultDefinition);
});

Expand Down Expand Up @@ -819,7 +826,6 @@ var addBehaviorToDefinition = function(definition, behavior, value) {
// Currently, this is adding default behavior
// copying `type` over, and even cleaning up the final definition object
makeDefinition = function(prop, def, defaultDefinition/*, typePrototype*/) {

var definition = {};

canReflect.eachKey(def, function(value, behavior) {
Expand Down Expand Up @@ -882,7 +888,7 @@ getDefinitionOrMethod = function(prop, value, defaultDefinition, typePrototype){
}
else if(typeof value === "function") {
if(canReflect.isConstructorLike(value)) {
definition = {Type: value};
definition = {Type: dataTypes.check(value)};
}
// or leaves as a function
} else if( Array.isArray(value) ) {
Expand Down Expand Up @@ -1027,10 +1033,14 @@ define.setup = function(props, sealed) {

/* jshint -W030 */

var requiredButNotProvided = new Set(this._define.required);
var definitions = this._define.definitions;
var instanceDefinitions = Object.create(null);
var map = this;
canReflect.eachKey(props, function(value, prop){
if(requiredButNotProvided.has(prop)) {
requiredButNotProvided.delete(prop);
}
if(definitions[prop] !== undefined) {
map[prop] = value;
} else {
Expand All @@ -1040,6 +1050,16 @@ define.setup = function(props, sealed) {
if(canReflect.size(instanceDefinitions) > 0) {
defineConfigurableAndNotEnumerable(this, "_instanceDefinitions", instanceDefinitions);
}
if(requiredButNotProvided.size) {
var msg, missingProps = Array.from(requiredButNotProvided);
if(requiredButNotProvided.size === 1) {
msg = `Missing required property [${missingProps[0]}].`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add the name of the type to this error message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure that we should, the required property is unrelated to the type. A property can be required without having an explicit type. I think adding the type might make it confusing, but I also see how it might be useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I meant the class, like canReflect.getName(this).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see, so if you are rendering a template with many components you can figure out which one it throwing the error? Makes sense. Probably should check for localName possibly even.

} else {
msg = `Missing required properties [${missingProps.join(", ")}].`;
}

throw new Error(msg);
}
// only seal in dev mode for performance reasons.
//!steal-remove-start
if(process.env.NODE_ENV !== 'production') {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
},
"dependencies": {
"can-assign": "^1.3.1",
"can-data-types": "^1.2.0",
"can-data-types": "1.3.0-pre.1",
"can-define-lazy-value": "^1.1.0",
"can-event-queue": "^1.1.5",
"can-log": "^1.0.0",
Expand Down
2 changes: 1 addition & 1 deletion test/define-class-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const QUnit = require("steal-qunit");
const Defined = require("../can-define-class");
const canReflect = require("can-reflect");

QUnit.module("DefineClass");
QUnit.module("can-define-class - DefineClass");

QUnit.test("Can define stuff", function(assert) {
class Faves extends Defined {
Expand Down
2 changes: 1 addition & 1 deletion test/define-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const QUnit = require("steal-qunit");
const { define } = require("../can-define-class");

QUnit.module("define()");
QUnit.module("can-define-class - define()");

QUnit.test("Can define stuff", function(assert) {
class Faves extends define() {
Expand Down
Loading