search

magesuite / magepack

Next generation Magento 2 advanced JavaScript bundler.
Open Software License 3.0
431 stars 93 forks source link

Remove unneeded additional RequireJS context #167

Closed fredden closed 1 year ago

fredden commented 1 year ago

Following #151, the additional RequireJS context seems unnecessary. From the limited testing that I have been able to carry out, unbundledContext.require.toUrl() behaves the same as require.toUrl() with those assets blocked / not loaded. This pull request simplifies the logic in lib/generate/collectModules.js.

krzksz commented 1 year ago

I've just realised our blocking may only work when people have JS merging disabled in the admin panel. Did you have a chance to test that?

czw., 27 paź 2022, 15:17 użytkownik Dan Wallis @.***> napisał:

Following #151 https://github.com/magesuite/magepack/pull/151, the additional RequireJS context seems unnecessary. From the limited testing that I have been able to carry out, unbundledContext.require.toUrl() behaves the same as require.toUrl() with those assets blocked / not loaded. This pull request simplifies the logic in lib/generate/collectModules.js.

You can view, comment on, or merge this pull request online at:

https://github.com/magesuite/magepack/pull/167 Commit Summary

File Changes

(1 file https://github.com/magesuite/magepack/pull/167/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/magesuite/magepack/pull/167, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGKOBJI7WPN6JQ5QKQ5ZQ3WFJ6IHANCNFSM6AAAAAARQBUYNY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

fredden commented 1 year ago

Good suggestion; I've not tested with that feature enabled. I'll test that next and let you know how I get on.

fredden commented 1 year ago

I can confirm that this additional RequireJS context is needed when JavaScript merging is enabled in Magento. The changes introduced in #151 are not enough to block all of Magepack when this feature is enabled.