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: lazy load GifUtil rather than nextTick #26

Merged
merged 1 commit into from Mar 12, 2022

Conversation

SimenB
Copy link
Contributor

@SimenB SimenB commented Sep 29, 2020

Inspired by the lazy helper in babel: https://github.com/babel/babel/blob/e498bee10f0123bb208baa228ce6417542a2c3c4/packages/babel-plugin-transform-modules-commonjs/src/index.js#L200-L207

The process.nextTick approach prints warnings when using Jest.

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down.

      at node_modules/gifwrap/src/gifcodec.js:7:15

The real solution here is to split up the files to avoid any dependency cycles, but this works as well without being async

@SimenB
Copy link
Contributor Author

SimenB commented Oct 26, 2020

ping @jtlapp

@AlcorSalvador
Copy link
Contributor

Dead Repo

@liquidg3
Copy link

liquidg3 commented Dec 7, 2020

Any chance this can get in?

@DavidVaness
Copy link

DavidVaness commented Jun 28, 2021

ping @jtlapp

edit:
I applied this change with the help of
https://www.npmjs.com/package/patch-package
instead of forking and it works well.
Thank you @SimenB

@axe312ger
Copy link

@jtlapp this would save a lot of people from headache when writing tests. Please merge and publish.

If you need any help doing so, let us know. Happy to help and give insights why this fix would be beneficial :)

@jtlapp jtlapp merged commit bb8c979 into jimp-dev:master Mar 12, 2022
@jtlapp
Copy link
Collaborator

jtlapp commented Mar 12, 2022

Okay done. I haven't verified the push, but I'll jump in if people report problems.

@jtlapp
Copy link
Collaborator

jtlapp commented Mar 12, 2022

All tests pass. Published to NPM.

@SimenB SimenB deleted the patch-1 branch March 13, 2022 10:26
@axe312ger
Copy link

Thanks!

@DavidVaness
Copy link

worked, thank you

@Marsup Marsup mentioned this pull request Apr 18, 2023
4 tasks
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.

None yet

6 participants