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

var idle is minified to var $ witch breaks a legacy Zepto(jQuery alternative) implementation #33

Closed
vascoeduardo opened this issue Oct 11, 2023 · 6 comments · Fixed by #37
Assignees
Labels
bug Something isn't working

Comments

@vascoeduardo
Copy link

Hi Accudio,

I use async-alpine on my web project that uses ZeptoJS(jQuery alternative) for a legacy feature. As Zepto and jQuery use $ as the main identifier my old code stops working, because your idle function is called $ in the minified version since async-alpine v1.1.0.

Is it possible to minify to another name.
Thanks for the great package and your help
Vasco

@Accudio
Copy link
Owner

Accudio commented Oct 11, 2023

Hey! Could you confirm how you're loading Async Alpine?

I'd much prefer to instead work out if we can scope and isolate Async Alpine to not leak and interact with any external scripts, instead of manipulating the minified output.

@Accudio Accudio added the bug Something isn't working label Oct 11, 2023
@Accudio Accudio self-assigned this Oct 11, 2023
@vascoeduardo
Copy link
Author

Hey, at the moment I am loading it with a <script> tag in the page head

<script defer src="https://www.myfantasydomain.com/assets/js/vendor/async-alpine/dist/async-alpine.script.js"></script>

I have a local copy of async-alpine.script.js not using the cdn

@Accudio
Copy link
Owner

Accudio commented Oct 11, 2023

Okay great, thanks for that. Due to workload I won't be able to take a look into this right now, but should be in the next couple of weeks.

In the meantime you could consider manually changing the variable name or adding type="module" to the script element may be enough to make it work.

@vascoeduardo
Copy link
Author

Hey Accudio,
we just changed the implementation. I have installed the npm module and using "import AsyncAlpine from 'async-alpine'" now everything works fine.
Thanks for you support

@iniznet
Copy link
Contributor

iniznet commented Oct 25, 2023

I've run into the same problem while managing an older site. I gotta say, it's a bit of a tricky issue because esbuild uses $ as one of its minification identifiers. The simplest solution would be to avoid exposing strategies globally. This way, we can minimize the likelihood of esbuild renaming them to $.

What do you think, Accudio? You can check out my commit attempt here:
iniznet@467c51d

I think we can use IIFE to isolate, but I decided to switch to npm+esm for better future.

@Accudio
Copy link
Owner

Accudio commented Oct 28, 2023

@iniznet That looks good and I'm happy to switch to using default exports. Although that fixes this particular problem of $, the script output does still pollute the global with single-letter variable names. That could be a problem in future so I think it makes sense to switch that to using an IIFE as you suggest.

That should be a simple change to line 12 of scripts/build.js, changing the fallback format to iife.
Would you be up for making that change and filing a PR?

Thank you!

iniznet added a commit to iniznet/async-alpine that referenced this issue Oct 29, 2023
@Accudio Accudio mentioned this issue Dec 13, 2023
Accudio added a commit that referenced this issue Dec 13, 2023
* feat: alias function module loading

* Enable multiple [name] replacements in AsyncAlpine.alias() (#34)

* Use IIFE format for the CDN build. Fixes #33

* Minor formatting tweaks

* Bumped version to 1.2.0

---------

Co-authored-by: Jon Whitter <[email protected]>
Co-authored-by: Andy Dennis <[email protected]>
Co-authored-by: Yusuf Fauzan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants