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

Static forDesignTime #19

Merged
merged 4 commits into from Apr 15, 2019

Conversation

Projects
None yet
2 participants
@Igor1201
Copy link
Owner

Igor1201 commented Apr 14, 2019

  • Make forDesignTime a class method, not factory;
  • forDesignTime can now return either Widget or List<Widget>;

vmaraccini and others added some commits Apr 9, 2019

@Igor1201

This comment has been minimized.

Copy link
Owner Author

Igor1201 commented Apr 14, 2019

@vmaraccini WDYT?

@vmaraccini
Copy link
Contributor

vmaraccini left a comment

Solution looks good, but the type checking seems a little brittle 😕
Do you know any APIs that can make it a stronger check? I'll try and search as well!

final double boundaryWidth =
annotatedElement.annotation.peek('width').doubleValue;
final double boundaryHeight =
annotatedElement.annotation.peek('height').doubleValue;

String constructor = '[${element.name}()]';
if (isForDesignTimeDefined &&
forDesignTime.returnType.displayName == 'Widget') {

This comment has been minimized.

Copy link
@vmaraccini

vmaraccini Apr 14, 2019

Contributor

Is there a better way to type-check this? I'm thinking that returning a concrete Widget subclass from the method will make this check fail, but there really is no reason for it.

Is there a good API to check if the class can be casted to or inherits from another one?

This comment has been minimized.

Copy link
@Igor1201

Igor1201 Apr 15, 2019

Author Owner

I'm not following, Widget seems right here, why do you want to be more specific?

This comment has been minimized.

Copy link
@vmaraccini

vmaraccini Apr 15, 2019

Contributor

If I do:

static MyWidgetWithRequiredProps forDesignTime() =>
       MyWidgetWithRequiredProps('Default test-only label.');

instead of:

static Widget forDesignTime() =>
       MyWidgetWithRequiredProps('Default test-only label.');

This check will fail, right?

This comment has been minimized.

Copy link
@Igor1201

Igor1201 Apr 15, 2019

Author Owner

Yes, you're right. But we have element: if we discover how to check the typing right, we can use it to check.

forDesignTime.returnType.displayName == 'Widget') {
constructor = '[${element.name}.forDesignTime()]';
} else if (isForDesignTimeDefined &&
forDesignTime.returnType.displayName == 'List<Widget>') {

This comment has been minimized.

Copy link
@vmaraccini

vmaraccini Apr 14, 2019

Contributor

Same argument applies here, since we could have a List<SomeSubclass>

@Igor1201

This comment has been minimized.

Copy link
Owner Author

Igor1201 commented Apr 15, 2019

@vmaraccini Yeah, I don't like this solution either; but I couldn't found a better way to check.
forDesignTime.returnType is a DartType, but I none of the checking methods I've tried worked for comparing with a Type.

@vmaraccini

This comment has been minimized.

Copy link
Contributor

vmaraccini commented Apr 15, 2019

Let's :shipit: then and find a way to fix it later ™️

@Igor1201 Igor1201 merged commit bf1f23f into master Apr 15, 2019

2 checks passed

ci/circleci: deps Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details

@Igor1201 Igor1201 deleted the list-fordesigntime branch Apr 15, 2019

@Igor1201

This comment has been minimized.

Copy link
Owner Author

Igor1201 commented Apr 15, 2019

Released as 1.0.0-alpha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.