magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.5k stars 9.3k forks source link

Global Dependency on window.checkoutConfig #7475

Closed astorm closed 7 years ago

astorm commented 7 years ago

Preconditions

  1. Install a Magento 2.1.x system.
  2. PHP/MySQL version irrelevant

Steps to reproduce

  1. Run the following javascript code on Magento's home page via any means you like
  2. `requirejs(['Magento_Checkout/js/model/url-builder'],function(){});

Expected result

  1. Code runs without errors

Actual result

  1. Code produces url-builder.js:12 Uncaught TypeError: Cannot read property 'storeCode' of undefined(…) error

This is one specific example of a more widespread problem. Many of Magento 2's RequireJS modules have a dependency on a global window.checkoutConfig object. This renders these modules unusable anywhere other than the Magento checkout pages.

If Magento's hewing closely to the the implicit guidelines they've set for themselves and their third party partners and independent open source developers working with the platform, this checkout configuration should be passed in to the RequireJS programs via an x-magento-init script.

You can see a list of these global dependencies with the following unix command line (replace vendor/magento with wherever your components are)

$ find vendor/magento/ -name '*.js' | xargs grep 'window\.checkoutConfig'

vendor/magento//module-braintree/view/frontend/web/js/view/payment/adapter.js:            return window.checkoutConfig.payment[this.getCode()].clientToken;
vendor/magento//module-braintree/view/frontend/web/js/view/payment/braintree.js:        var config = window.checkoutConfig.payment,
vendor/magento//module-braintree/view/frontend/web/js/view/payment/method-renderer/cc-form.js:                validator.setConfig(window.checkoutConfig.payment[this.getCode()]);
vendor/magento//module-braintree/view/frontend/web/js/view/payment/method-renderer/cc-form.js:                return window.checkoutConfig.payment[this.getCode()].hasFraudProtection;
vendor/magento//module-braintree/view/frontend/web/js/view/payment/method-renderer/cc-form.js:                return window.checkoutConfig.payment[this.getCode()].environment;
vendor/magento//module-braintree/view/frontend/web/js/view/payment/method-renderer/cc-form.js:                return window.checkoutConfig.payment[this.getCode()].kountMerchantId;
vendor/magento//module-braintree/view/frontend/web/js/view/payment/method-renderer/hosted-fields.js:            return window.checkoutConfig.payment[this.getCode()].ccVaultCode;
vendor/magento//module-braintree/view/frontend/web/js/view/payment/method-renderer/paypal.js:            return window.checkoutConfig.payment[this.getCode()].title;
vendor/magento//module-braintree/view/frontend/web/js/view/payment/method-renderer/paypal.js:            return window.checkoutConfig.payment[this.getCode()].locale;
vendor/magento//module-braintree/view/frontend/web/js/view/payment/method-renderer/paypal.js:            return window.checkoutConfig.payment[this.getCode()].isAllowShippingAddressOverride;
vendor/magento//module-braintree/view/frontend/web/js/view/payment/method-renderer/paypal.js:            return window.checkoutConfig.payment[this.getCode()].merchantName;
vendor/magento//module-braintree/view/frontend/web/js/view/payment/method-renderer/paypal.js:            return window.checkoutConfig.payment[this.getCode()].paymentAcceptanceMarkSrc;
vendor/magento//module-braintree/view/frontend/web/js/view/payment/validator-handler.js:            return window.checkoutConfig.payment;
vendor/magento//module-checkout/view/frontend/web/js/action/redirect-on-success.js:            redirectUrl: window.checkoutConfig.defaultSuccessPageUrl,
vendor/magento//module-checkout/view/frontend/web/js/model/checkout-data-resolver.js:                if (!availableRate && window.checkoutConfig.selectedShippingMethod) {
vendor/magento//module-checkout/view/frontend/web/js/model/checkout-data-resolver.js:                    selectShippingMethodAction(window.checkoutConfig.selectedShippingMethod);
vendor/magento//module-checkout/view/frontend/web/js/model/new-customer-address.js:            countryId: (addressData.country_id) ? addressData.country_id : window.checkoutConfig.defaultCountryId,
vendor/magento//module-checkout/view/frontend/web/js/model/new-customer-address.js:                : window.checkoutConfig.defaultRegionId,
vendor/magento//module-checkout/view/frontend/web/js/model/new-customer-address.js:            postcode: addressData.postcode ? addressData.postcode : window.checkoutConfig.defaultPostcode,
vendor/magento//module-checkout/view/frontend/web/js/model/postcode-validator.js:            var patterns = window.checkoutConfig.postCodes[countryId];
vendor/magento//module-checkout/view/frontend/web/js/model/quote.js:        var quoteData = window.checkoutConfig.quoteData;
vendor/magento//module-checkout/view/frontend/web/js/model/quote.js:        var basePriceFormat = window.checkoutConfig.basePriceFormat;
vendor/magento//module-checkout/view/frontend/web/js/model/quote.js:        var priceFormat = window.checkoutConfig.priceFormat;
vendor/magento//module-checkout/view/frontend/web/js/model/quote.js:        var storeCode = window.checkoutConfig.storeCode;
vendor/magento//module-checkout/view/frontend/web/js/model/quote.js:        var totalsData = window.checkoutConfig.totalsData;
vendor/magento//module-checkout/view/frontend/web/js/model/quote.js:                return window.checkoutConfig.quoteItemData;
vendor/magento//module-checkout/view/frontend/web/js/model/shipping-rates-validation-rules.js:            checkoutConfig = window.checkoutConfig;
vendor/magento//module-checkout/view/frontend/web/js/model/shipping-rates-validator.js:        var checkoutConfig = window.checkoutConfig,
vendor/magento//module-checkout/view/frontend/web/js/model/step-navigator.js:                    window.location.href = window.checkoutConfig.pageNotFoundUrl;
vendor/magento//module-checkout/view/frontend/web/js/model/step-navigator.js:                            window.location = window.checkoutConfig.checkoutUrl + "#" + code;
vendor/magento//module-checkout/view/frontend/web/js/model/step-navigator.js:                    window.location = window.checkoutConfig.checkoutUrl + "#" + code;
vendor/magento//module-checkout/view/frontend/web/js/model/url-builder.js:            storeCode: window.checkoutConfig.storeCode,
vendor/magento//module-checkout/view/frontend/web/js/view/authentication.js:        var checkoutConfig = window.checkoutConfig;
vendor/magento//module-checkout/view/frontend/web/js/view/billing-address.js:                    if (window.checkoutConfig.reloadOnBillingAddress) {
vendor/magento//module-checkout/view/frontend/web/js/view/billing-address.js:                    if (window.checkoutConfig.reloadOnBillingAddress) {
vendor/magento//module-checkout/view/frontend/web/js/view/billing-address.js:                        if (window.checkoutConfig.reloadOnBillingAddress) {
vendor/magento//module-checkout/view/frontend/web/js/view/form/element/email.js:        forgotPasswordUrl: window.checkoutConfig.forgotPasswordUrl,
vendor/magento//module-checkout/view/frontend/web/js/view/payment.js:        paymentService.setPaymentMethods(methodConverter(window.checkoutConfig.paymentMethods));
vendor/magento//module-checkout/view/frontend/web/js/view/payment.js:                return window.checkoutConfig.formKey;
vendor/magento//module-checkout/view/frontend/web/js/view/summary/item/details/thumbnail.js:        var imageData = window.checkoutConfig.imageData;
vendor/magento//module-checkout-agreements/view/frontend/web/js/model/agreement-validator.js:        var checkoutConfig = window.checkoutConfig,
vendor/magento//module-checkout-agreements/view/frontend/web/js/model/agreements-assigner.js:    var agreementsConfig = window.checkoutConfig.checkoutAgreements;
vendor/magento//module-checkout-agreements/view/frontend/web/js/view/checkout-agreements.js:        var checkoutConfig = window.checkoutConfig,
vendor/magento//module-offline-payments/view/frontend/web/js/view/payment/method-renderer/banktransfer-method.js:                return window.checkoutConfig.payment.instructions[this.item.method];
vendor/magento//module-offline-payments/view/frontend/web/js/view/payment/method-renderer/cashondelivery-method.js:                return window.checkoutConfig.payment.instructions[this.item.method];
vendor/magento//module-offline-payments/view/frontend/web/js/view/payment/method-renderer/checkmo-method.js:                return window.checkoutConfig.payment.checkmo.mailingAddress;
vendor/magento//module-offline-payments/view/frontend/web/js/view/payment/method-renderer/checkmo-method.js:                return window.checkoutConfig.payment.checkmo.payableTo;
vendor/magento//module-payment/view/frontend/web/js/view/payment/cc-form.js:                return window.checkoutConfig.payment.ccform.availableTypes[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/cc-form.js:                return window.checkoutConfig.payment.ccform.icons.hasOwnProperty(type) ?
vendor/magento//module-payment/view/frontend/web/js/view/payment/cc-form.js:                    window.checkoutConfig.payment.ccform.icons[type]
vendor/magento//module-payment/view/frontend/web/js/view/payment/cc-form.js:                return window.checkoutConfig.payment.ccform.months[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/cc-form.js:                return window.checkoutConfig.payment.ccform.years[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/cc-form.js:                return window.checkoutConfig.payment.ccform.hasVerification[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/cc-form.js:                return window.checkoutConfig.payment.ccform.hasSsCardType[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/cc-form.js:                return window.checkoutConfig.payment.ccform.cvvImageUrl[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/cc-form.js:                return window.checkoutConfig.payment.ccform.ssStartYears[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/iframe.js:                return window.checkoutConfig.payment.iframe.source[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/iframe.js:                return window.checkoutConfig.payment.iframe.controllerName[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/iframe.js:                return window.checkoutConfig.payment.iframe.placeOrderUrl[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/iframe.js:                return window.checkoutConfig.payment.iframe.cgiUrl[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/iframe.js:                return window.checkoutConfig.payment.iframe.saveOrderUrl[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/iframe.js:                return window.checkoutConfig.payment.iframe.dateDelim[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/iframe.js:                return window.checkoutConfig.payment.iframe.cardFieldsMap[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/iframe.js:                return window.checkoutConfig.payment.iframe.expireYearLength[this.getCode()];
vendor/magento//module-payment/view/frontend/web/js/view/payment/iframe.js:                return window.checkoutConfig.payment.iframe.timeoutTime[this.getCode()];
vendor/magento//module-paypal/view/frontend/web/js/view/payment/method-renderer/iframe-methods.js:                return this.isInAction() ? window.checkoutConfig.payment.paypalIframe.actionUrl[this.getCode()] : '';
vendor/magento//module-paypal/view/frontend/web/js/view/payment/method-renderer/paypal-billing-agreement.js:                return window.checkoutConfig.payment.paypalBillingAgreement.transportName;
vendor/magento//module-paypal/view/frontend/web/js/view/payment/method-renderer/paypal-billing-agreement.js:                return window.checkoutConfig.payment.paypalBillingAgreement.agreements;
vendor/magento//module-paypal/view/frontend/web/js/view/payment/method-renderer/paypal-express-abstract.js:                return window.checkoutConfig.payment.paypalExpress.paymentAcceptanceMarkHref;
vendor/magento//module-paypal/view/frontend/web/js/view/payment/method-renderer/paypal-express-abstract.js:                return window.checkoutConfig.payment.paypalExpress.paymentAcceptanceMarkSrc;
vendor/magento//module-paypal/view/frontend/web/js/view/payment/method-renderer/paypal-express-abstract.js:                return window.checkoutConfig.payment.paypalExpress.billingAgreementCode[this.item.method];
vendor/magento//module-paypal/view/frontend/web/js/view/payment/method-renderer/paypal-express-abstract.js:                                window.checkoutConfig.payment.paypalExpress.redirectUrl[quote.paymentMethod().method]
vendor/magento//module-paypal/view/frontend/web/js/view/payment/paypal-payments.js:        var isContextCheckout = window.checkoutConfig.payment.paypalExpress.isContextCheckout,
vendor/magento//module-paypal/view/frontend/web/js/view/payment/paypal-payments.js:                config: window.checkoutConfig.payment.paypalExpress.inContextConfig
vendor/magento//module-paypal/view/frontend/web/js/view/review/actions/iframe.js:                return this.isInAction() ? window.checkoutConfig.payment.paypalIframe.actionUrl[this.getCode()] : '';
vendor/magento//module-persistent/view/frontend/web/js/view/remember-me.js:        var persistenceConfig = window.checkoutConfig.persistenceConfig;
vendor/magento//module-shipping/view/frontend/web/js/model/config.js:            return window.checkoutConfig.shippingPolicy
vendor/magento//module-tax/view/frontend/web/js/view/checkout/cart/totals/tax.js:        var isFullTaxSummaryDisplayed = window.checkoutConfig.isFullTaxSummaryDisplayed,
vendor/magento//module-tax/view/frontend/web/js/view/checkout/cart/totals/tax.js:            isZeroTaxDisplayed = window.checkoutConfig.isZeroTaxDisplayed;
vendor/magento//module-tax/view/frontend/web/js/view/checkout/shipping_method/price.js:            isDisplayShippingPriceExclTax: window.checkoutConfig.isDisplayShippingPriceExclTax,
vendor/magento//module-tax/view/frontend/web/js/view/checkout/shipping_method/price.js:            isDisplayShippingBothPrices: window.checkoutConfig.isDisplayShippingBothPrices,
vendor/magento//module-tax/view/frontend/web/js/view/checkout/summary/grand-total.js:                isFullTaxSummaryDisplayed: window.checkoutConfig.isFullTaxSummaryDisplayed || false,
vendor/magento//module-tax/view/frontend/web/js/view/checkout/summary/grand-total.js:            isTaxDisplayedInGrandTotal: window.checkoutConfig.includeTaxInGrandTotal || false,
vendor/magento//module-tax/view/frontend/web/js/view/checkout/summary/item/details/subtotal.js:        var displayPriceMode = window.checkoutConfig.reviewItemPriceDisplayMode || 'including';
vendor/magento//module-tax/view/frontend/web/js/view/checkout/summary/shipping.js:        var displayMode = window.checkoutConfig.reviewShippingDisplayMode;
vendor/magento//module-tax/view/frontend/web/js/view/checkout/summary/subtotal.js:        var displaySubtotalMode = window.checkoutConfig.reviewTotalsDisplayMode;
vendor/magento//module-tax/view/frontend/web/js/view/checkout/summary/tax.js:        var isTaxDisplayedInGrandTotal = window.checkoutConfig.includeTaxInGrandTotal;
vendor/magento//module-tax/view/frontend/web/js/view/checkout/summary/tax.js:        var isFullTaxSummaryDisplayed = window.checkoutConfig.isFullTaxSummaryDisplayed;
vendor/magento//module-tax/view/frontend/web/js/view/checkout/summary/tax.js:        var isZeroTaxDisplayed = window.checkoutConfig.isZeroTaxDisplayed;
vendor/magento//module-usps/view/frontend/web/js/model/shipping-rates-validator.js:        var checkoutConfig = window.checkoutConfig;
vendor/magento//module-vault/view/frontend/web/js/view/payment/method-renderer/vault.js:                return window.checkoutConfig.payment.ccform.icons.hasOwnProperty(type) ?
vendor/magento//module-vault/view/frontend/web/js/view/payment/method-renderer/vault.js:                    window.checkoutConfig.payment.ccform.icons[type]
vendor/magento//module-vault/view/frontend/web/js/view/payment/vault-enabler.js:                return typeof window.checkoutConfig.vault[this.paymentCode] !== 'undefined' &&
vendor/magento//module-vault/view/frontend/web/js/view/payment/vault-enabler.js:                    window.checkoutConfig.vault[this.paymentCode]['is_enabled'] === true;
vendor/magento//module-vault/view/frontend/web/js/view/payment/vault.js:        _.each(window.checkoutConfig.payment.vault, function (config, index) {
vendor/magento//module-weee/view/frontend/web/js/view/checkout/summary/item/price/row_excl_tax.js:                if (!window.checkoutConfig.getIncludeWeeeFlag) {
vendor/magento//module-weee/view/frontend/web/js/view/checkout/summary/item/price/row_excl_tax.js:                if (window.checkoutConfig.getIncludeWeeeFlag) {
vendor/magento//module-weee/view/frontend/web/js/view/checkout/summary/item/price/row_incl_tax.js:                if (!window.checkoutConfig.getIncludeWeeeFlag) {
vendor/magento//module-weee/view/frontend/web/js/view/checkout/summary/item/price/row_incl_tax.js:                if (window.checkoutConfig.getIncludeWeeeFlag) {
vendor/magento//module-weee/view/frontend/web/js/view/checkout/summary/item/price/weee.js:                return window.checkoutConfig.isDisplayPriceWithWeeeDetails;
vendor/magento//module-weee/view/frontend/web/js/view/checkout/summary/item/price/weee.js:                return window.checkoutConfig.isDisplayFinalPrice;
vendor/magento//module-weee/view/frontend/web/js/view/checkout/summary/weee.js:            isIncludedInSubtotal: window.checkoutConfig.isIncludedInSubtotal,
SerhiyShkolyarenko commented 7 years ago

@astorm thank you for the report!

The URL builder you run was written for checkout only. It was not implied to be run on other pages, so it won't work on home page. window.checkoutConfig is data container for initializing checkout specific JS components, so your grep command just enlisted components initialized on checkout page.

Please feel free to submit your improvement ideas to the new Magento 2 Feature Requests and Improvements forum (see details here).

astorm commented 7 years ago

@SerhiyShkolyarenko @piotrekkaminski This isn't a new feature request, it's a bug report. Also, it's not unconfirmed, it's clearly there. I hope Magento reconsiders its decision not to fix this bug in their RequireJS modules. Irrespective of

The URL builder you run was written for checkout only

the fact other developers can pull this module into their own code means they will pull it into their own code, and that dependency means they'll waste time trying to figure out what's going on (I know I did). Outside developers don't have access to Magento Inc.'s list of assumptions. Also, by having this bug remain in place, it will encourage third party developers to rely on global state in their own code and modules.

Pulling these dependencies into a x-magento-init script would (seem to?) solve this problem.

I get that, politically speaking, it's easier to punt on something like this but it's a problem that should be fixed. Again, I hope you reconsider your decision.

SerhiyShkolyarenko commented 7 years ago

@astorm sorry I didn't understand your proposal on utilizing x-magento-init.

General info: components in folders lib/web and app/code/Magento/Ui are global and may be utilized on any page. All other JS components are module specific and should be utilized when you develop extension to a specific module. For example you may utilize all the components from your list on checkout page, but not on catalog category page or home page.

antonkril commented 7 years ago

@astorm, agree. window.checkoutConfig configuration violates our guidelines. There is a task to fix it. But it was prioritized low when checkout was implemented.

Your request will bump its priority. MAGETWO-61548

astorm commented 7 years ago

Thank you @antonkril.

@SerhiyShkolyarenko @antonkril Moved discussion of "best practices" to forum: https://community.magento.com/t5/Programming-Questions/Clarifying-Unstated-Assumptions-for-RequireJS-Modules/m-p/54228

vdubyna commented 7 years ago

@astorm, @antonkril We detected the same problem on 3g connection (to replicate, you can switch the connection in developer tools) We've fixed it by adding 'domReady!' (http://requirejs.org/docs/api.html#pageload) dependency to the libraries which depend on the source code. Example: vendor/magento/module-checkout/view/frontend/web/js/model/quote.js So, after the fix, it looks like this define( ['ko', 'domReady!'], function (ko) { //... });

antonkril commented 7 years ago

@vdubyna, in stock Magento vendor/magento/module-checkout/view/frontend/web/js/model/quote.js is loaded by x-magento-init, which is executed after DOM is loaded (see magento2ce/lib/web/mage/bootstrap.js:19). Do you have any customisations that call quote.js before DOM is loaded, or on pages that do not have window.checkoutConfig

vdubyna commented 7 years ago

@antonkril I'll verify customizations. I've checked pure magento and could not recreate the problem. Though the page is not as large as on the project. Let you know about the results.

veloraven commented 7 years ago

This issue is treated as an improvement, not a bug. New internal ticket - MAGETWO-65583

mihaifaget commented 7 years ago

Even on checkout page, sometimes checkoutConfig loads later than any of those depending it and checkout fails. I'd still call it bug

piotrekkaminski commented 7 years ago

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.

Khaleel commented 6 years ago

This is still in the new releases and at a loss at how these developers from Ukraine operate. Complete no regards to FE and basic UI and UX requirements. No wonder Magento are going with PWA and scrapping this idea of over engineered solutions.