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: upgrade xmlbuilder2 to solve unhandled rejection error #168

Merged

Conversation

zedtux
Copy link
Contributor

@zedtux zedtux commented Nov 16, 2022

This PR fixes the error reported from issue #92.

After having found this issue from the xmlbuilder2 library, I tried to update the (quite) old version of xmlbuilder2 this package use and the .docx generation now work in my project !

@privateOmega
Copy link
Owner

privateOmega commented Nov 17, 2022

@zedtux TYSM for this PR. I know this has been a headache for a couple of users, would really like to merge this PR in ASAP, but before that could you please rebase your base branch with develop, and target to the develop branch in this repo as well?

@zedtux zedtux changed the base branch from master to develop November 17, 2022 08:13
@zedtux zedtux force-pushed the upgrades-xmlbuilder2-version branch from 26ef94f to dfe5e9a Compare November 17, 2022 09:12
@zedtux
Copy link
Contributor Author

zedtux commented Nov 17, 2022

Done.

You should set the develop branch as the default one if it is. I got my PR pointing master since it is the default branch.

@privateOmega
Copy link
Owner

Done.

You should set the develop branch as the default one if it is. I got my PR pointing master since it is the default branch.

Thanks, that was intentional since, sometimes there would be some time difference between when a PR lands on develop branch and when I do a release, and develop is a branch I dont mind having in a broken state.

@zedtux
Copy link
Contributor Author

zedtux commented Nov 17, 2022

I see, but then it leads to the mistake I did.

@privateOmega privateOmega changed the title Upgrades the xmlbuilder2 dependency version fix: upgrade xmlbuilder2 to solve unhandled rejection error Nov 17, 2022
@privateOmega privateOmega merged commit 108e1e9 into privateOmega:develop Nov 17, 2022
@zedtux zedtux deleted the upgrades-xmlbuilder2-version branch November 17, 2022 18:22
@privateOmega
Copy link
Owner

@zedtux Funstuff, I just remembered why I hard fixed xmlbuilder2 version to 2.1.2, xmlbuilder2 v2.1.3 introduces a bug which makes the generated docx unusable.

image

So if you notice the wp:inline somehow changed to w:inline which is not supposed to be.

I just triaged it, and it seems to be introduced by a fix for oozcitak/xmlbuilder2#18. So I will be reverting your changes.

@privateOmega
Copy link
Owner

#171 Raised and merged this now to solve the "unhandled rejection error" by forcing the respective transitive deps, and not updating xmlbuilder2 to the buggy versions.

@zedtux
Copy link
Contributor Author

zedtux commented Nov 17, 2022

@privateOmega It doesn't work with version 1.6.2 :

Uncaught (in promise) TypeError: Cannot call a class as a function
    _classCallCheck NodeImpl.js:11
    NodeImpl NodeImpl.js:66
    _createSuperInternal DocumentImpl.js:29
    DocumentImpl DocumentImpl.js:80
    ...

@zedtux
Copy link
Contributor Author

zedtux commented Nov 17, 2022

Also, you're sticking to a very old version of xmlbuilder2. Could you please check newer versions ?

@privateOmega
Copy link
Owner

@zedtux its supposed to be 1.6.3, not 1.6.2. Well I tried with multiple versions of xmlbuilder including latest, but it breaks the image feature which I cannot allow. I have posted a comment in xmlbuilder2 to know if there;s a workaround, if yes I will upgrade it for sure.

@nsantini
Copy link

@privateOmega this still doesn't work on version 1.6.5 :(

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.

3 participants