Upcoming breaking changes (v17)

[emmenko]

This issue aims to give an overview of all the breaking changes we would like to introduce in the next major version v17.

• [Breaking] Migrate packages to new @apollo/client #1677 Migrate to Apollo v3
• [Breaking] refactor: remove mc-scripts extract-intl, document usage of @formatjs/cli command #1790 Remove the extract-intl command
  • Remove the babel plugin for intl
• [Breaking] refactor: drop mc-http-server, make --use-local-assets the default behavior #1787 Make the --use-local-assets the default behavior
  • The usage of this flag is confusing and it turns out that this is what you need 99% of the time so we should make it the default behavior.
  • Remove the mc-http-server.
• [Breaking] refactor(app-shell): remove experimentalRenderAppWithRedux method, use disableApolloMocks instead #1791 Remove experimentalRenderAppWithRedux, instead expose a RealApolloProvider component wrapper that can be passed as ApolloProviderComponent option to renderApp and renderAppWithRedux
• Remove deprecated CLI options

  • merchant-center-application-kit/packages/mc-html-template/src/compile-html.js

    Line 9 in 96b3af7

    const deprecatedOptions = ['envPath', 'cspPath', 'headersPath'];

  • https://github.com/commercetools/merchant-center-application-kit/blob/master/packages/application-config/src/load-deprecated-config.ts
• [Breaking] chore(application-shell): remove deprecated components. #1781 Remove deprecated exports:

  • merchant-center-application-kit/packages/application-shell/src/index.ts

    Lines 21 to 23 in 74d43d2

    	// Deprecated
    	export { default as AsyncChunkLoader } from './components/async-chunk-loader';
    	export { default as handleApolloErrors } from './components/handle-apollo-errors';

• [Breaking] refactor(application-shell): remove support for trackingEventWhitelist #1780 Remove deprecated options, due to oppressive language renaming.

  • merchant-center-application-kit/packages/application-shell/src/components/application-shell/application-shell.tsx

    Lines 77 to 79 in 564cd91

    	// This is deprecated, to avoid usage of oppressive language!
    	// Use `trackingEventList` instead.
    	trackingEventWhitelist?: TrackingList;

[tdeekens]

Remove deprecated hostnames

I am not sure we can do that yet. Accordingly to my knowledge customers are still relying on them. The hostname migration date at least is postponed until January next year. We can remove them here if there is no change for old published versions though.

React Router to v6 + History to v5

I fear this would entail a lot of work internally. Maybe we can smoothen the migration out internally but evaluating changes we need to make first.

Remove experimentalRenderAppWithRedux, instead expose a RealApolloProvider component wrapper that can be passed as ApolloProviderComponent option to renderApp and renderAppWithRedux

Would it possible to do this in a two step migration. First expose the wrapper and allow passing it. Recommend to migrate until the then next major v18 and only then remove it?

[adnasa]

I agree with Tobi's points above.

I personally feel that this is a lot of work for a single major version.
We can for example go with removing experimentalRenderAppWithRedux and that itself is a breaking change on its own.
Same for the Apollo v3 migration and react router.

Can we break them down to different releases/version instead of one?
I think we should align the release strategy here and involve PM

[emmenko]

I fear this would entail a lot of work internally.

It is a lot of work yes. I just mentioned it as optional but more likely we won't be able to do it yet. React router v6 is still in beta also, so we should wait for a stable release at least.

Would it possible to do this in a two step migration. First expose the wrapper and allow passing it. Recommend to migrate until the then next major v18 and only then remove it?

The migration would be very simple. But also given that the method is called "experimental" I would just remove it without worrying much about it.

[tdeekens]

React router v6 is still in beta also, so we should wait for a stable release at least.

Yeah I just wonder if we can refactor out app to be as compatible as possible already.

The migration would be very simple. But also given that the method is called "experimental" I would just remove it without worrying much about it.

Fine by me. Just wanted to mention that we can also add a step to smoothen things internally.

[emmenko]

@adnasa only the stuff marked as optional is more work, which is why is listed as optional.
The rest (besides the apollo migration which we already finished like 80%) is just removing stuff that was basically deprecated.

So I wouldn't split that stuff.

[adnasa]

Remove the extract-intl command

I'm not sure what the plan is and if we should do this. Please elaborate

My guess is that we want to remove this because it is assumed that it is not used. MC-FE is still using this command. If we are sure (and data supports it) that this isn't needed by external custom app devs, we can maybe move this script into the MC-FE repo.

[emmenko]

I'm not sure what the plan is and if we should do this. Please elaborate

The new @formatjs/cli kind of replaces our custom script. Appkit and uikit already use the new CLI, so in theory we should be able to use that for our internal apps as well.
Therefore, the extract-intl command is not necessary anymore, especially from a Custom Application point of view.

So what we can do is remove the command in our internal apps first, just to make sure everything works. But from a Custom Application point of view it shouldn't be necessary anymore.

[adnasa]

ah okay. clear, thanks for clarifying

[adnasa]

We can remove #1773 from the check list above. I don't think that should be blocking a v17 release.
I realised now that be possible to migrate sdk-get away from enzyme without having to use msw for it.

[emmenko]

I moved the enzyme task to optional for the release.
We should plan it better, for example by allowing users to still opt-in even if we remove it from our tooling.

[emmenko]

We worked on all the important breaking changes. I'll work on the release notes to have it ready for release.

[tdeekens]

Would you want to remove the disabledMenuItems in this release to or wouldn't you consider it breaking as it's an undocumented API and it can be removed later at any point?

[emmenko]

We can remove it I suppose yes. We don't need to document it in the release notes then.

Do you want to open a PR?

[adnasa]

I don't think the disabledMenuItems needs to be documented

[tdeekens]

I don't think the disabledMenuItems needs to be documented

🕊️ Sounds good to me. Just wanted to mention it ☮️

[emmenko]

Closing this. The leftover items have been moved to a new issue: #1829