babel-plugin-i18next-extract icon indicating copy to clipboard operation
babel-plugin-i18next-extract copied to clipboard

Fix empty transpiled file cache issue

Open xu3u4 opened this issue 3 years ago • 2 comments

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 https://github.com/babel/babel/discussions/13590#discussioncomment-1034864 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 avatar Jul 31 '22 08:07 xu3u4

@gilbsgilbs Please check it when you have time 🙏

xu3u4 avatar Aug 02 '22 05:08 xu3u4

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?

gilbsgilbs avatar Aug 02 '22 08:08 gilbsgilbs

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

ITJesse avatar Aug 09 '22 21:08 ITJesse

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 avatar Aug 10 '22 19:08 gilbsgilbs

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

gilbsgilbs avatar Aug 10 '22 19:08 gilbsgilbs

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.

xu3u4 avatar Aug 12 '22 09:08 xu3u4