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

fix: extract works with template string id's #1027

Merged
merged 1 commit into from
Apr 5, 2021
Merged

Conversation

sultan99
Copy link
Contributor

@sultan99 sultan99 commented Apr 4, 2021

This PR closes issue #957 macro t with template string id.

@vercel
Copy link

vercel bot commented Apr 4, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/lingui-js/js-lingui/2J7df8q4wwA5mtL9S1CUN9RAeqgz
✅ Preview: https://js-lingui-git-fork-sultan99-main-lingui-js.vercel.app

@codecov
Copy link

codecov bot commented Apr 4, 2021

Codecov Report

Merging #1027 (c8d3206) into main (1069b15) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1027      +/-   ##
==========================================
+ Coverage   83.35%   83.36%   +0.01%     
==========================================
  Files          53       53              
  Lines        1622     1623       +1     
  Branches      436      438       +2     
==========================================
+ Hits         1352     1353       +1     
  Misses        157      157              
  Partials      113      113              
Impacted Files Coverage Δ
...ackages/babel-plugin-extract-messages/src/index.ts 82.40% <100.00%> (+0.14%) ⬆️

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 c567ea4...c8d3206. Read the comment docs.

@semoal
Copy link
Contributor

semoal commented Apr 5, 2021

Looks good mate, but could you me explain what does it mean?
value?.quasis[0]?.value?.cooked, just for giving context to the pull request

@sultan99
Copy link
Contributor Author

sultan99 commented Apr 5, 2021

@semoal ? - It is an optional chaining operator, so with ? we don't get ReferenceError error if any value in the object will be undefined. I did it to cover all cases for the test coverage 😅.

If you are asking about this object value.quasis[0].value.cooked, so the value in the template string is stored in that path and I had to get it in order to make it all work.

@semoal
Copy link
Contributor

semoal commented Apr 5, 2021

@semoal ? - It is an optional chaining operator, so with ? we don't get ReferenceError error if any value in the object will be undefined. I did it to cover all cases for the test coverage 😅.

If you are asking about this object value.quasis[0].value.cooked, so the value in the template string is stored in that path and I had to get it in order to make it all work.

Yes, i was asking about the object quasis and the cooked value. Also, another question, shouldn't be like this?:

props[key.name] = (isIdLiteral && value?.quasis[0]?.value?.cooked) || value.value || undefined

When is a template literal always value.value it's undefined?
Just being sure that we don't break anything ^^

@sultan99
Copy link
Contributor Author

sultan99 commented Apr 5, 2021

Well, there is no big difference, but anyway I have optimized it.
Please check it & let's publish it! 🤘

@semoal semoal changed the title fix(babel-plugin-extract-messages): extraction template string id fix: extract works with template string id's Apr 5, 2021
@semoal semoal merged commit a17d629 into lingui:main Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants