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

Unexpected token #231

Closed
tfrijsewijk opened this issue Apr 19, 2021 · 5 comments
Closed

Unexpected token #231

tfrijsewijk opened this issue Apr 19, 2021 · 5 comments
Labels

Comments

@tfrijsewijk
Copy link

tfrijsewijk commented Apr 19, 2021

Hi,

Great lib! Looks promising. I'm developing a Leaflet plugin and I need to import some library from node_modules for that.

I receive the following error:

SyntaxError: Unexpected token (1434:11) in xxx\core\dist\esm\index-65dcb9fa.js

Line 1434:

    return import(

It looks like klap has issues with the lazy import statement when an a node_module has import(), if I rename it to something else all is fine (but I'm getting runtime errors, obviously).

The imported code is ESM, do you have any idea why klap can't handle this?

Thanks!

@tfrijsewijk
Copy link
Author

So after extensive debugging I narrowed it down to a commented word (process) 🤯

I even made a demo repo: https://github.com/tfrijsewijk/klap-test

In the original source code the comment was:

// connectedCallback will be processed once all components have been registered

The problem is the word process. If you rename it to Process, strip of the s, whatever -> it works! The comment is from a library I use, which in turn is generated code (Stencil), so I removed it using patch-package because I don't have control over it.

But holy smokes! This is weird

@tfrijsewijk
Copy link
Author

tfrijsewijk commented Apr 21, 2021

So I went ahead and cloned klap and built it locally. After some binary commenting in ./plugins.js I found out that removing nodeGlobals(), fixes the error...

So there's another clue

edit: Changing it to nodeGlobals({ process: false }), also works. According to https://github.com/calvinmetcalf/rollup-plugin-node-globals#rollup-plugin-node-globals this will disable inlining the process variable.

I don't know if I want that or not, but I'm a bit at a loss here..

@osdevisnot
Copy link
Owner

👋 @tfrijsewijk - my apologies for getting back little late on this one, and thank you so much for clear and minimal re-production of the original issue.

You are also right in narrowing this down to rollup-plugin-node-globals. I tried this with simplified rollup build here and was able to re-produce the issue. This however works ok if you remove the dynamic import from loader.js

At this point, I am unsure why this might be the case. We should create an issue with node-globals repo for now to get some feedback.

@osdevisnot
Copy link
Owner

🎉 This issue has been resolved in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tfrijsewijk
Copy link
Author

Hi @osdevisnot - Don't mention it and thanks for resolving it. I'm upgrading right away :-)

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

2 participants