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 Viewer.js breaking import #1434

Merged
merged 1 commit into from
Apr 10, 2024
Merged

FIX Viewer.js breaking import #1434

merged 1 commit into from
Apr 10, 2024

Conversation

Kurtil
Copy link
Contributor

@Kurtil Kurtil commented Apr 10, 2024

This PR reverts this import to its previous value. The current import is breaking xeokit if used as an npm dependency.

@xeolabs xeolabs merged commit 7dcd3a6 into xeokit:master Apr 10, 2024
1 of 2 checks passed
@xeolabs xeolabs added this to the 2.6.0 milestone Apr 10, 2024
@xeolabs
Copy link
Member

xeolabs commented Apr 10, 2024

@paireks
Copy link
Member

paireks commented Apr 21, 2024

@Kurtil Hello, thank you for correcting me :) Question is if there is a third option, because everytime I want to develop by importing Viewer.js like this: import {Viewer} from "../../src/viewer/Viewer.js"
I have an error:
2024-04-21_12h29_14
And I have to manually switch it to this other type of import inside Viewer.js file, and later remove it. Do you have an idea how to solve this, so both this and the npm dependency will work?

@Kurtil Kurtil deleted the patch-17 branch April 22, 2024 07:24
@Kurtil
Copy link
Contributor Author

Kurtil commented Apr 22, 2024

@paireks I had the same issue. This issue is due to the fact that this import is not a valid ESM javascript import, it is an import that must be handled first by a bundler.

I imagine you serve directly your file using a local web server without bundling it like I do (seeing that we have this same error). In this case, you can comment the html2canvas import line if you do not use the corresponding lines like getSnapshot if I remember correctly, or a nice solution I use now is to use import map in the head of the html file you serve:

<script type="importmap">
  {
    "imports": {
      "html2canvas/dist/html2canvas.esm.js": "https://cdn.jsdelivr.net/npm/html2canvas@1.4.1/dist/html2canvas.esm.js"
    }
  }
</script>

@xeolabs
Copy link
Member

xeolabs commented Apr 22, 2024

@Kurtil @paireks Would it make sense to extend the build process to copy html2canvas from node_modules into the ./src directory? Then it becomes part of the source tree.

@Kurtil
Copy link
Contributor Author

Kurtil commented Apr 22, 2024

Resolve the path using a bundler is more "js friendly" I assume...

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

3 participants