Skip to content
This repository has been archived by the owner on Nov 27, 2022. It is now read-only.

Support LDflex expressions resulting in unthenable. #17

Merged
merged 2 commits into from
Aug 27, 2019

Conversation

satotake
Copy link
Contributor

@satotake satotake commented Apr 30, 2019

Kind of change

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Proposed changes

This fixation can actually eliminate error described in #12 but I doubt if its result is what you expect.

Relevant issues

#12

Breaking change?

  • Yes
  • No

Requirements check

  • All tests are passing
  • New/updated tests are included

Additional info

This fixation can actually eliminate the above error but I doubt if its result is what you expect.

For example,

<Value src="[https://ruben.verborgh.org/profile/#me]"/>

Rendered as

"https://ruben.verborgh.org/profile/#me"

Any comments and suggestion will be appreciated.

@coveralls
Copy link

coveralls commented Apr 30, 2019

Pull Request Test Coverage Report for Build 132

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 130: 0.0%
Covered Lines: 272
Relevant Lines: 272

💛 - Coveralls

@RubenVerborgh
Copy link
Owner

Thanks!

I'm thinking that we should be open to any kind of objects, since there is no hard test to check whether an object is an LDflex expression. So perhaps, if an object is not thenable, we should just return the object itself.

@satotake
Copy link
Contributor Author

satotake commented May 6, 2019

Helpful response. Thanks!

In that case, all we need is to just remove resolveExpression in ExpressionEvaluator.

Then, above jsx-expressions are converted into below elements.

<p> <Value src="[https://ruben.verborgh.org/profile/#me]"/> </p>
<p> <Value src="[https://this.is.invalid/#expression"/> </p>
<p> <Value src={1234} /> </p>
<p> https://ruben.verborgh.org/profile/#me </p>
<p> <span class="solid value error" data-error="Expression &quot;[https://this.is.invalid/#expression &quot; is invalid: Unexpected token :"></span> </p>
<p> 1234 </p>

In addition, some tests are fixed.

Sorry that I failed to use PR template. so I updated my top comment.

@satotake
Copy link
Contributor Author

satotake commented May 6, 2019

After all, instead removing resolveExpression itself, I removed error check in resolveExpression.

@RubenVerborgh RubenVerborgh self-assigned this Aug 27, 2019
@RubenVerborgh RubenVerborgh added the enhancement New feature or request label Aug 27, 2019
@RubenVerborgh
Copy link
Owner

Apologies for the extreme delay, and thanks again for your contribution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants