Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

chore(getElementType): remove unused logic branches#1396

Merged
kuzhelov merged 5 commits intomasterfrom
chore/remove-unused-logic-of-get-element-type
May 28, 2019
Merged

chore(getElementType): remove unused logic branches#1396
kuzhelov merged 5 commits intomasterfrom
chore/remove-unused-logic-of-get-element-type

Conversation

@kuzhelov
Copy link
Contributor

@kuzhelov kuzhelov commented May 27, 2019

This PR removes couple of branches from logic of getElementType method that infers element type for rendered Stardust component.

One of these branches - getDefault, - wasn't used anywhere, while another one provides unnecessary complication to element type's inference.

BREAKING CHANGES

These changes are NOT breaking for clients that are using Typescript, as well as highly likely to introduce no breaking changes to JS clients, given that they are affecting edge-case usages.

The following elements that were originally rendered as a now will be rendered as as or default element of the corresponding component:

// - was rendered as <a href>{content}</a> 
// - now is rendered as <span href>{content}</span> 
<Text href='..' content={...} />  

Migration path

It is necessary to provide value for as prop explicitly:

<Text as='a' href='..' content={...} /> 

@kuzhelov kuzhelov changed the title getElementType: remove unused logic branches chore(getElementType): remove unused logic branches May 27, 2019
Copy link
Member

@miroslavstastny miroslavstastny left a comment

Choose a reason for hiding this comment

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

👍 on code changes.
But as this functionality goes back to SUIR, it might be useful to hear @levithomason opinion.

@codecov
Copy link

codecov bot commented May 27, 2019

Codecov Report

Merging #1396 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1396      +/-   ##
==========================================
+ Coverage   73.47%   73.49%   +0.01%     
==========================================
  Files         778      778              
  Lines        5859     5855       -4     
  Branches     1726     1723       -3     
==========================================
- Hits         4305     4303       -2     
+ Misses       1548     1546       -2     
  Partials        6        6
Impacted Files Coverage Δ
packages/react/src/lib/getElementType.tsx 100% <100%> (+25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1283e9...167bf32. Read the comment docs.

@codecov
Copy link

codecov bot commented May 27, 2019

Codecov Report

Merging #1396 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1396      +/-   ##
==========================================
+ Coverage   73.49%   73.51%   +0.01%     
==========================================
  Files         786      786              
  Lines        5867     5863       -4     
  Branches     1706     1723      +17     
==========================================
- Hits         4312     4310       -2     
+ Misses       1549     1547       -2     
  Partials        6        6
Impacted Files Coverage Δ
packages/react/src/lib/getElementType.tsx 100% <100%> (+25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8a7e6e...f9e1764. Read the comment docs.

@kuzhelov kuzhelov merged commit ee49fc9 into master May 28, 2019
@delete-merged-branch delete-merged-branch bot deleted the chore/remove-unused-logic-of-get-element-type branch May 28, 2019 09:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants