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.

Sign up for GitHub

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

Jump to bottom

matrix-widget-api triggers soft crash in Safari 13.1 #15493

Closed
jryans opened this issue Oct 19, 2020 · 1 comment · Fixed by matrix-org/matrix-widget-api#6
Closed

matrix-widget-api triggers soft crash in Safari 13.1 #15493

jryans opened this issue Oct 19, 2020 · 1 comment · Fixed by matrix-org/matrix-widget-api#6
Assignees
@jryans
Labels
A-Widgets T-Defect X-Release-Blocker Z-Platform-Specific

Comments

@jryans
Copy link
Collaborator

jryans commented Oct 19, 2020

The issue here seems to be roughly the following:

  1. ClientWidgetApi from the new matrix-widget-api added in Element 1.7.9 extends AlmostEventEmitter which extends EventTarget
  2. The compiled code for class extension in matrix-widget-api uses Reflect.construct to extend a super class
  3. In Safari 13.1 (the previous major release), Reflect.construct(EventTarget, [...]) fails with Reflect.construct requires the first argument be a constructor

Since it's the previous major release, it's still within our support window, so we should try to concoct some kind of fix here. Options that come to mind include:

  • Change how matrix-widget-api compiles class extension
  • Stop extending EventTarget
@jryans jryans self-assigned this Oct 19, 2020
@jryans
Copy link
Collaborator Author

jryans commented Oct 19, 2020

Changing to class compilation does not help, a similar error happens with native classes also, seems like EventTarget as a constructor was just not supported until Safari 14. (We should still change code generation for general sanity, but it won't help for this problem.)

jryans added a commit to matrix-org/matrix-widget-api that referenced this issue Oct 19, 2020
@jryans
Use same Babel targets as other libraries
508a40e 
This signal to Babel that most modern syntax is safe to use, resulting in much
smaller, easier to read compiled output.

Noticed while investigating element-hq/element-web#15493
jryans added a commit to matrix-org/matrix-widget-api that referenced this issue Oct 19, 2020
@jryans
Use same Babel targets as other libraries
5f24d20 
This signals to Babel that most modern syntax is safe to use, resulting in much
smaller, easier to read compiled output.

Noticed while investigating element-hq/element-web#15493
@jryans jryans mentioned this issue Oct 19, 2020
Closed
jryans added a commit to matrix-org/matrix-widget-api that referenced this issue Oct 19, 2020
@jryans
Change to events package for better compatibility
f678cda 
This moves away from using `EventTarget` directly, as that is not available in
various environments such as Safari 13.1. As a replacement, this changes to
`events` which emulates the Node.js API in browser.

This is a breaking change, so clients will need to adjust code that adds
listeners to call the new function names.

Fixes element-hq/element-web#15493
jryans added a commit to matrix-org/matrix-react-sdk that referenced this issue Oct 19, 2020
@jryans
Adjust for new widget messaging APIs
7fe98b4 
As part of changing to the `events` package, the API surface changed slightly.

Related to element-hq/element-web#15493
jryans added a commit that referenced this issue Oct 19, 2020
@jryans
Adjust for new widget messaging APIs
dfb1579 
As part of changing to the `events` package, the API surface changed slightly.

Related to #15493
This was referenced Oct 19, 2020
Merged
Merged
Merged
jryans added a commit to matrix-org/matrix-react-sdk that referenced this issue Oct 19, 2020
@jryans
Adjust for new widget messaging APIs
11199f8 
As part of changing to the `events` package, the API surface changed slightly.

Related to element-hq/element-web#15493
jryans added a commit that referenced this issue Oct 19, 2020
@jryans
Adjust for new widget messaging APIs
ea49858 
As part of changing to the `events` package, the API surface changed slightly.

Related to #15493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jryans jryans

Labels
A-Widgets T-Defect X-Release-Blocker Z-Platform-Specific
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Change to events package for better compatibility matrix-org/matrix-widget-api
1 participant
@jryans