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.

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

upgrade: rusty_v8 0.19.0 #9466

Merged
merged 11 commits into from
Feb 15, 2021
Merged

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented Feb 10, 2021

Actual ICU implementation will follow in a separate PR

@bartlomieju bartlomieju requested a review from ry February 10, 2021 21:09
@ry ry changed the title upgrade: rusty_v8 0.18.0 upgrade: rusty_v8 0.18.2 Feb 11, 2021
@kitsonk
Copy link
Contributor

kitsonk commented Feb 11, 2021

This will allow the tests to pass, but I think something is wrong with ICU in Deno:

diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js
index 50631e83..a1ea5612 100644
--- a/cli/tsc/99_main_compiler.js
+++ b/cli/tsc/99_main_compiler.js
@@ -8,10 +8,25 @@
 // that is created when Deno needs to type check TypeScript, and in some
 // instances convert TypeScript to JavaScript.
 
+"use strict";
+
 // Removes the `__proto__` for security reasons.  This intentionally makes
 // Deno non compliant with ECMA-262 Annex B.2.2.1
 delete Object.prototype.__proto__;
 
+// TODO(@kitsonk) this is hacky and shouldn't be here at all...
+Intl.Collator = (() => {
+  function Collator() {}
+
+  Collator.prototype = {
+    compare(a, b) {
+      return a < b ? -1 : a > b ? 1 : 0;
+    }
+  }
+
+  return Collator;
+})();
+
 ((window) => {
   /** @type {DenoCore} */
   const core = window.Deno.core;

@kitsonk
Copy link
Contributor

kitsonk commented Feb 11, 2021

Ah, yeah, didn't pay attention that the ICU implementation happens later... yeah, maybe it is just best to ignore the tests as is currently done until ICU is implemented and then re-enable them.

@ry
Copy link
Member

ry commented Feb 11, 2021

Intl.Collator is not working. I'm investigating it in denoland/rusty_v8#617

@ry
Copy link
Member

ry commented Feb 12, 2021

The Intl.Collator issue should be resolved now. The problem was that the ICU data needs to be 16-bit aligned.

Happy to land this, but ICU should probably wait for 1.8.0? Maybe we should wait until after 1.7.3 is released.

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bartlomieju
Copy link
Member Author

Waiting on v0.19.0 of rusty_v8 before landing this one

@bartlomieju bartlomieju changed the title upgrade: rusty_v8 0.18.2 upgrade: rusty_v8 0.19.0 Feb 15, 2021
@bartlomieju bartlomieju merged commit 0cf952e into denoland:master Feb 15, 2021
@bartlomieju bartlomieju deleted the upgrade_rusty_v8 branch February 15, 2021 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants