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

Use native XML implementation in browsers #330

Closed
awerlang opened this issue Jul 28, 2017 · 7 comments
Closed

Use native XML implementation in browsers #330

awerlang opened this issue Jul 28, 2017 · 7 comments
Labels

Comments

@awerlang
Copy link

Environment

  • Version of docxtemplater : 3.1.3
  • Used docxtemplater-modules : [email protected]
  • Runner : Browser

How to reproduce my problem :

  • Install docxtemplater
  • Write an ES6 module on top of it
  • Bundle with rollup with default settings

On iOS, it will fail with "SyntaxError: In strict mode code, functions can only be declared at top level or immediately within another function."

This is due syntax used on xmldom, which isn't a problem there, but turns out to cause bundles to fail when compiled with "use strict". A PR from 6 months ago is waiting to be merged jindw/xmldom#194.

I could disable "use strict" on rollup, but it wouldn't be optimal. I could fork xmldom as well, but perhaps we don't really need it on browser?

So here's the request:

Is it possible to build a browser version using native DOMParser instead of xmldom? The trouble is that this package fails in a strict environment. It is also unmaintained.

docxtemplater-image-module would also need a browser bundle.

Or perhaps there's a reason to use xmldom even on browser?

Looking forward to your thoughts. Thanks!

@edi9999
Copy link
Member

edi9999 commented Aug 11, 2017

Yes, I agree that xmldom is not maintained well enough. For node environment, we would need an alternative either way. I haven't expected any other issues with it for now, and I think the code is quite good, but the real problem is that there is no maintainer. This is also a reason why I don't like to have too many dependencies (projects are very often created but not maintained over the long run) : we only have xmldom and jszip as a real dependency.

One point to use xmldom instead DomParser in browsers is that we know that xmldom will behave in the same way in all browsers, whereas they might be subtle differences with different browsers (I'm looking at IE and Safari).

@awerlang
Copy link
Author

I replaced rollup with webpack, as it deals with non-strict commonjs safely.

This is another interesting PR on xmldom, although it may not be merged for the very reason you stated above: jindw/xmldom#152

There are other ways to do it, at least in rollup it's possible to provide a replacement for any imported module, so this could be handled on a per-project basis.

Perhaps the current release of xmldom won't cause further problems, but I'd say you should be prepared to maintain a separate fork of xmldom any day.

@frederikbosch
Copy link

frederikbosch commented Sep 6, 2017

What about https://github.com/tmpvar/jsdom. Native implementation for browser, jsdom for node.

@edi9999
Copy link
Member

edi9999 commented Sep 8, 2017

Is there a way to import only what we need ? eg DOMParser, XMLSerializer

@frederikbosch
Copy link

Maybe by only using one or more of its dependencies.

@edi9999
Copy link
Member

edi9999 commented Sep 16, 2017

I forgot that when I created the minified files, I have already done what you mention :

https://github.com/open-xml-templating/docxtemplater/blob/master/create-min.js

xmldom() {
    return {
        XMLSerializer: window.XMLSerializer,
        DOMParser: window.DOMParser,
    };
},

@edi9999
Copy link
Member

edi9999 commented Nov 11, 2017

I have now documented how to minify the build with an alias for xmldom (for webpack)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants