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 empty transpiled file cache issue #227

Merged
merged 2 commits into from
Aug 10, 2022

Conversation

xu3u4
Copy link
Contributor

@xu3u4 xu3u4 commented Jul 31, 2022

To fix #208
I found the exporterCache object doesn't persist across all the runs in newer babel versions(7.16.10+).
After some runs, it becomes empty.
The solution is inspired by babel/babel#13590 (comment)
It points out that the cache object needs to be put outside the plugin function. (maybe we can also consider to use weakMap for the performance improvement.)

post() {
      const extractState = this.I18NextExtract;
      if (extractState.extractedKeys.length === 0) return;
      console.log("exporterCache")
      console.log(exporterCache)
      // ... rest
},
Before After
image image

Btw, thank you for the awesome plugin, it really helps our i18n flow a lot by reducing many manual steps! 🌟 🌟 🌟

@xu3u4 xu3u4 changed the title Fix transpiled file cache Fix missing transpiled file cache issue Jul 31, 2022
@xu3u4 xu3u4 changed the title Fix missing transpiled file cache issue Fix empty transpiled file cache issue Jul 31, 2022
@xu3u4
Copy link
Contributor Author

xu3u4 commented Aug 2, 2022

@gilbsgilbs Please check it when you have time 🙏

@gilbsgilbs
Copy link
Owner

gilbsgilbs commented Aug 2, 2022

Thanks for your contribution.

maybe we can also consider to use weakMap for the performance improvement

I'm not sure I understand this part. What would be the keys of such weakMap? Instances of BabelCore.ConfigAPI? If that works, I think I'd prefer that over plain globals, but I suspect babel wont even pass consistent instances of ConfigAPI across runs 😕 . It's not much about performance though.

Also, do you know at which point this broke? Or was it always broken somehow?

Did you try with the latest release candidate + completely removing and recreating your lockfile and node_modules?

@ITJesse
Copy link

ITJesse commented Aug 9, 2022

I can confirm that this patch works great. Without this, the output will always be the last file.
This problem is also reported here: #208

@gilbsgilbs gilbsgilbs force-pushed the fix-transpiled-file-cache branch from bac0bc1 to 6b8aee7 Compare August 10, 2022 19:18
@gilbsgilbs
Copy link
Owner

gilbsgilbs commented Aug 10, 2022

I just checked, and the api object indeed changes across calls (so I really don't understand how a weakmap is supposed to help here). I think the most sensible thing to do would be to create a cache per package.json (by using "root" or finding the closest to the source file using pkg-up or something) and fallback to a global cache only in last resort. But given nobody may actually have a use-case for this and as discardOldKeys is just broken at the moment, I'll consider this out of scope for this PR.

Added a commit with miscellaneous improvements not really related to your PR while I was playing around.

Thanks for your contribution @xu3u4 🙏 .

@gilbsgilbs gilbsgilbs merged commit 8ea0e74 into gilbsgilbs:master Aug 10, 2022
@gilbsgilbs
Copy link
Owner

Published under 0.9.0-rc.1 . I'll make a final release at some point.

@xu3u4
Copy link
Contributor Author

xu3u4 commented Aug 12, 2022

Thank you very much! 🙏
Sorry for the late reply. I was busy with other stuff and haven't got time to check it carefully.
Looking forward for the new release!

so I really don't understand how a weakmap is supposed to help here

Sorry it may not fit this case as the keys are string instead of object. But I guess the suggestion is trying to say that weakMap element can be garbage collected if the key is removed.

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.

Using discardOldKeys keeps only strings from last file
3 participants