prosemirror/prosemirror-menu
1
0
Fork 0
Code Issues 7 Pull requests Activity

Update crel to v4.2.0, which supports IE11 #25

Closed
pocke wants to merge 1 commit from update-crel-to-v4.2 into master
pull from: update-crel-to-v4.2
merge into: prosemirror:master
Conversation 4 Commits 1 Files changed 1 +1 -1
pocke commented 2019-12-02 15:02:51 +01:00 (Migrated from github.com)
Copy link

ref: 71e13c5e10
https://github.com/ProseMirror/prosemirror/issues/1009
https://github.com/KoryNunn/crel/pull/55

crel v4.1.0 does not works well with IE11, but crel v4.2.0 works by https://github.com/KoryNunn/crel/pull/55.

So we can update the dependency.

ref: 71e13c5e10a3284afe12a9ac3e520fd566a01724 https://github.com/ProseMirror/prosemirror/issues/1009 https://github.com/KoryNunn/crel/pull/55 crel v4.1.0 does not works well with IE11, but crel v4.2.0 works by https://github.com/KoryNunn/crel/pull/55. So we can update the dependency.
marijnh commented 2019-12-02 15:16:38 +01:00 (Migrated from github.com)
Copy link

I'm still seeing an unconditional call to new Proxy in the code, which is what caused https://github.com/ProseMirror/prosemirror/issues/1009

I'm still seeing an unconditional call to `new Proxy` in the code, which is what caused https://github.com/ProseMirror/prosemirror/issues/1009
pocke commented 2019-12-02 15:47:03 +01:00 (Migrated from github.com)
Copy link

Thanks, I overlooked this error. 🙇‍♂️

I guess we can upgrade the dependency if the user installs the polyfill. https://github.com/GoogleChrome/proxy-polyfill
crel v4.2.0 provides code that is valid as ES5 syntax, so we can avoid the error with the polyfill.

But I'm not sure which is a better way, v3 without polyfill or v4 with polyfill. If you'd like to avoid adding the polyfill to the dependencies, feel free to close this pull request.

Thanks!

Thanks, I overlooked this error. :bowing_man: I guess we can upgrade the dependency if the user installs the polyfill. https://github.com/GoogleChrome/proxy-polyfill crel v4.2.0 provides code that is valid as ES5 syntax, so we can avoid the error with the polyfill. But I'm not sure which is a better way, v3 without polyfill or v4 with polyfill. If you'd like to avoid adding the polyfill to the dependencies, feel free to close this pull request. Thanks!
marijnh commented 2019-12-02 16:05:50 +01:00 (Migrated from github.com)
Copy link

I'm definitely not introducing a dependency on a polyfill just to be able to upgrade another dependency. But maybe the crel maintainers can be convinced to reinstate the if (typeof Proxy == "undefined") line, since the issue you linked suggests they do still target older browsers. Though the situation seems a bit of a mess—there's an es.js file compiled town to ES5 and referenced in package.json as module, but the main file, crel.js is using ES6 syntax.

I'm definitely not introducing a dependency on a polyfill just to be able to upgrade another dependency. But maybe the crel maintainers can be convinced to reinstate the `if (typeof Proxy == "undefined")` line, since the issue you linked suggests they do still target older browsers. Though the situation seems a bit of a mess—there's an `es.js` file compiled town to ES5 and referenced in package.json as `module`, _but_ the `main` file, `crel.js` is using ES6 syntax.
marijnh commented 2021-05-17 13:44:38 +02:00 (Migrated from github.com)
Copy link

(Moving to crelt instead of crel has made this moot.)

(Moving to `crelt` instead of `crel` has made this moot.)
❤️ 1

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
prosemirror/prosemirror-menu!25
No description provided.