Page MenuHomePhabricator
Differential D6704

Make react-icomoon work with Webpack 5
ClosedPublic
Actions

Authored by ashoat on Feb 12 2023, 7:03 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 12, 6:00 PM
Unknown Object (File)
Fri, Apr 11, 3:44 PM
Unknown Object (File)
Fri, Apr 11, 5:28 AM
Unknown Object (File)
Thu, Apr 10, 11:51 PM
Unknown Object (File)
Thu, Apr 10, 3:33 PM
Unknown Object (File)
Thu, Apr 10, 1:01 AM
Unknown Object (File)
Tue, Mar 25, 11:41 AM
Unknown Object (File)
Wed, Mar 19, 2:28 PM
View All Files
Subscribers
None

Details

Reviewers
atul
tomek
Commits
rCOMM08eb701d6c08: Make react-icomoon work with Webpack 5
Summary

react-icomoon has a quirky CommonJS export with a hack meant to make it work when imported from an ES Modules context. The idea was that would set exports.__esModule to enable the hack, and you would export a prop named default that would be the "default" import for the ESM.

This hack was supported in Webpack 4, but not in Webpack 5. There's a lot of noise about this on GitHub, but the Webpack team points to Node as no longer supported this hack either as justification for pulling it out.

The react-icomoon package uses this hack. I tried upgrading it, but the latest version still had the same problem. To get around it, I just update the code to access .default directly.

Note that this means lib/components/SWMansionIcon.react.js wouldn't work in a React Native context, despite react-icomoon claiming to support React Native. But we don't need it on React Native since we use createIconSetFromIcoMoon from @expo/vector-icons instead.

Depends on D6703

Test Plan
  1. I tested dev build in web via yarn dev in web + keyserver and then tested the website
  2. I tested prod build by running yarn prod in web and then tested the website by running keyserver with yarn prod-build && yarn-prod

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Revision Contents
Changeset List

PathSize
lib/
components/
5 lines
2 lines
web/
4 lines
2 lines
8 lines

Diff 22388

View Options

lib/components/SWMansionIcon.react.js

  • This file was added.
// @flow
import invariant from 'invariant';
import Alert from 'react-native/Libraries/Alert/Alert';
import {
sendReactionMessage,
sendReactionMessageActionTypes,
} from 'lib/actions/message-actions';
import type { BindServerCall, DispatchFunctions } from 'lib/utils/action-utils';
import type { TooltipRoute } from '../navigation/tooltip.react';
function onPressReact(
route:
| TooltipRoute<'TextMessageTooltipModal'>
| TooltipRoute<'MultimediaMessageTooltipModal'>,
dispatchFunctions: DispatchFunctions,
bindServerCall: BindServerCall,
) {
const messageID = route.params.item.messageInfo.id;
const threadID = route.params.item.threadInfo.id;
tomekUnsubmitted
Not Done
Inline Actions

This approach seems to be overly complicated. Can't we just write a function that receives messageID, threadID and reaction? This function can be named after an action that is performed, so react or sendReaction.

tomek: This approach seems to be overly complicated. Can't we just write a function that receives…
invariant(messageID, 'messageID should be set');
invariant(threadID, 'threadID should be set');
const callSendReactionMessage = bindServerCall(sendReactionMessage);
const reactionMessagePromise = (async () => {
try {
const result = await callSendReactionMessage(threadID, messageID, '👍');
return {
targetMessageID: messageID,
serverID: result.id,
threadID,
time: result.newMessageInfo.time,
interface: result.interface,
newMessageInfos: [result.newMessageInfo],
};
} catch (e) {
Alert.alert(
tomekUnsubmitted
Not Done
Inline Actions

It sounds risky to show one alert after every failed message. A user might react to a lot of messages in short period and a network issue could result in a lot of alerts being shown. Other apps sometimes choose to silently fail when a reaction fails.

tomek: It sounds risky to show one alert after every failed message. A user might react to a lot of…
ginsuAuthorUnsubmitted
Done
Inline Actions

Totally see your concern here! I spoke to @atul about this today, and we both agreed that we want this to be a temporary solution, but since reaction messages don't have retry behavior we think it is best to show some sort of feedback to the user in case something failed.

ginsu: Totally see your concern here! I spoke to @atul about this today, and we both agreed that we…
'Couldn’t send the reaction',
'Please try again later',
[{ text: 'OK' }],
{
cancelable: false,
tomekUnsubmitted
Not Done
Inline Actions

Is there a reason to set cancelable to false?

tomek: Is there a reason to set cancelable to false?
ginsuAuthorUnsubmitted
Done
Inline Actions

Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by default and can be enabled by providing an optional Options parameter with the cancelable property set to true. Here is more context

I also found the alert on report-utils to follow the same patern

ginsu: Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by…
},
);
throw e;
}
})();
dispatchFunctions.dispatchActionPromise(
sendReactionMessageActionTypes,
reactionMessagePromise,
);
}
export { onPressReact };
View Options

lib/package.json

  • This file was added.
// @flow
import invariant from 'invariant';
import Alert from 'react-native/Libraries/Alert/Alert';
import {
sendReactionMessage,
sendReactionMessageActionTypes,
} from 'lib/actions/message-actions';
import type { BindServerCall, DispatchFunctions } from 'lib/utils/action-utils';
import type { TooltipRoute } from '../navigation/tooltip.react';
function onPressReact(
route:
| TooltipRoute<'TextMessageTooltipModal'>
| TooltipRoute<'MultimediaMessageTooltipModal'>,
dispatchFunctions: DispatchFunctions,
bindServerCall: BindServerCall,
) {
const messageID = route.params.item.messageInfo.id;
const threadID = route.params.item.threadInfo.id;
tomekUnsubmitted
Not Done
Inline Actions

This approach seems to be overly complicated. Can't we just write a function that receives messageID, threadID and reaction? This function can be named after an action that is performed, so react or sendReaction.

tomek: This approach seems to be overly complicated. Can't we just write a function that receives…
invariant(messageID, 'messageID should be set');
invariant(threadID, 'threadID should be set');
const callSendReactionMessage = bindServerCall(sendReactionMessage);
const reactionMessagePromise = (async () => {
try {
const result = await callSendReactionMessage(threadID, messageID, '👍');
return {
targetMessageID: messageID,
serverID: result.id,
threadID,
time: result.newMessageInfo.time,
interface: result.interface,
newMessageInfos: [result.newMessageInfo],
};
} catch (e) {
Alert.alert(
tomekUnsubmitted
Not Done
Inline Actions

It sounds risky to show one alert after every failed message. A user might react to a lot of messages in short period and a network issue could result in a lot of alerts being shown. Other apps sometimes choose to silently fail when a reaction fails.

tomek: It sounds risky to show one alert after every failed message. A user might react to a lot of…
ginsuAuthorUnsubmitted
Done
Inline Actions

Totally see your concern here! I spoke to @atul about this today, and we both agreed that we want this to be a temporary solution, but since reaction messages don't have retry behavior we think it is best to show some sort of feedback to the user in case something failed.

ginsu: Totally see your concern here! I spoke to @atul about this today, and we both agreed that we…
'Couldn’t send the reaction',
'Please try again later',
[{ text: 'OK' }],
{
cancelable: false,
tomekUnsubmitted
Not Done
Inline Actions

Is there a reason to set cancelable to false?

tomek: Is there a reason to set cancelable to false?
ginsuAuthorUnsubmitted
Done
Inline Actions

Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by default and can be enabled by providing an optional Options parameter with the cancelable property set to true. Here is more context

I also found the alert on report-utils to follow the same patern

ginsu: Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by…
},
);
throw e;
}
})();
dispatchFunctions.dispatchActionPromise(
sendReactionMessageActionTypes,
reactionMessagePromise,
);
}
export { onPressReact };
View Options

web/CommIcon.react.js

  • This file was added.
// @flow
import invariant from 'invariant';
import Alert from 'react-native/Libraries/Alert/Alert';
import {
sendReactionMessage,
sendReactionMessageActionTypes,
} from 'lib/actions/message-actions';
import type { BindServerCall, DispatchFunctions } from 'lib/utils/action-utils';
import type { TooltipRoute } from '../navigation/tooltip.react';
function onPressReact(
route:
| TooltipRoute<'TextMessageTooltipModal'>
| TooltipRoute<'MultimediaMessageTooltipModal'>,
dispatchFunctions: DispatchFunctions,
bindServerCall: BindServerCall,
) {
const messageID = route.params.item.messageInfo.id;
const threadID = route.params.item.threadInfo.id;
tomekUnsubmitted
Not Done
Inline Actions

This approach seems to be overly complicated. Can't we just write a function that receives messageID, threadID and reaction? This function can be named after an action that is performed, so react or sendReaction.

tomek: This approach seems to be overly complicated. Can't we just write a function that receives…
invariant(messageID, 'messageID should be set');
invariant(threadID, 'threadID should be set');
const callSendReactionMessage = bindServerCall(sendReactionMessage);
const reactionMessagePromise = (async () => {
try {
const result = await callSendReactionMessage(threadID, messageID, '👍');
return {
targetMessageID: messageID,
serverID: result.id,
threadID,
time: result.newMessageInfo.time,
interface: result.interface,
newMessageInfos: [result.newMessageInfo],
};
} catch (e) {
Alert.alert(
tomekUnsubmitted
Not Done
Inline Actions

It sounds risky to show one alert after every failed message. A user might react to a lot of messages in short period and a network issue could result in a lot of alerts being shown. Other apps sometimes choose to silently fail when a reaction fails.

tomek: It sounds risky to show one alert after every failed message. A user might react to a lot of…
ginsuAuthorUnsubmitted
Done
Inline Actions

Totally see your concern here! I spoke to @atul about this today, and we both agreed that we want this to be a temporary solution, but since reaction messages don't have retry behavior we think it is best to show some sort of feedback to the user in case something failed.

ginsu: Totally see your concern here! I spoke to @atul about this today, and we both agreed that we…
'Couldn’t send the reaction',
'Please try again later',
[{ text: 'OK' }],
{
cancelable: false,
tomekUnsubmitted
Not Done
Inline Actions

Is there a reason to set cancelable to false?

tomek: Is there a reason to set cancelable to false?
ginsuAuthorUnsubmitted
Done
Inline Actions

Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by default and can be enabled by providing an optional Options parameter with the cancelable property set to true. Here is more context

I also found the alert on report-utils to follow the same patern

ginsu: Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by…
},
);
throw e;
}
})();
dispatchFunctions.dispatchActionPromise(
sendReactionMessageActionTypes,
reactionMessagePromise,
);
}
export { onPressReact };
View Options

web/package.json

  • This file was added.
// @flow
import invariant from 'invariant';
import Alert from 'react-native/Libraries/Alert/Alert';
import {
sendReactionMessage,
sendReactionMessageActionTypes,
} from 'lib/actions/message-actions';
import type { BindServerCall, DispatchFunctions } from 'lib/utils/action-utils';
import type { TooltipRoute } from '../navigation/tooltip.react';
function onPressReact(
route:
| TooltipRoute<'TextMessageTooltipModal'>
| TooltipRoute<'MultimediaMessageTooltipModal'>,
dispatchFunctions: DispatchFunctions,
bindServerCall: BindServerCall,
) {
const messageID = route.params.item.messageInfo.id;
const threadID = route.params.item.threadInfo.id;
tomekUnsubmitted
Not Done
Inline Actions

This approach seems to be overly complicated. Can't we just write a function that receives messageID, threadID and reaction? This function can be named after an action that is performed, so react or sendReaction.

tomek: This approach seems to be overly complicated. Can't we just write a function that receives…
invariant(messageID, 'messageID should be set');
invariant(threadID, 'threadID should be set');
const callSendReactionMessage = bindServerCall(sendReactionMessage);
const reactionMessagePromise = (async () => {
try {
const result = await callSendReactionMessage(threadID, messageID, '👍');
return {
targetMessageID: messageID,
serverID: result.id,
threadID,
time: result.newMessageInfo.time,
interface: result.interface,
newMessageInfos: [result.newMessageInfo],
};
} catch (e) {
Alert.alert(
tomekUnsubmitted
Not Done
Inline Actions

It sounds risky to show one alert after every failed message. A user might react to a lot of messages in short period and a network issue could result in a lot of alerts being shown. Other apps sometimes choose to silently fail when a reaction fails.

tomek: It sounds risky to show one alert after every failed message. A user might react to a lot of…
ginsuAuthorUnsubmitted
Done
Inline Actions

Totally see your concern here! I spoke to @atul about this today, and we both agreed that we want this to be a temporary solution, but since reaction messages don't have retry behavior we think it is best to show some sort of feedback to the user in case something failed.

ginsu: Totally see your concern here! I spoke to @atul about this today, and we both agreed that we…
'Couldn’t send the reaction',
'Please try again later',
[{ text: 'OK' }],
{
cancelable: false,
tomekUnsubmitted
Not Done
Inline Actions

Is there a reason to set cancelable to false?

tomek: Is there a reason to set cancelable to false?
ginsuAuthorUnsubmitted
Done
Inline Actions

Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by default and can be enabled by providing an optional Options parameter with the cancelable property set to true. Here is more context

I also found the alert on report-utils to follow the same patern

ginsu: Alerts on Android can be dismissed by tapping outside of the alert box. It is disabled by…
},
);
throw e;
}
})();
dispatchFunctions.dispatchActionPromise(
sendReactionMessageActionTypes,
reactionMessagePromise,
);
}
export { onPressReact };
View Options

yarn.lock

Loading...