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

Fix slow build issue #2543

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backend/push.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export const subscriptionInfoWrapper = (subcriptionId: string, subscriptionInfo:
let salt
let uaPublic

return function (this: Object) {
return function (/* this: Object */) {
Copy link
Member

Choose a reason for hiding this comment

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

Why was this commented out? 🤔

(I think @corrideat added this for some specific reason)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It caused a syntax error, complaining about function (this) being invalid code, like if only the : Object part had been removed when transpiling JS with Flow annotations to plain JS.

Copy link
Member

Choose a reason for hiding this comment

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

@snowteamer What complained about a syntax error? It's definitely a JavaScript syntax error (this isn't allowed as an identifier), but it's not a Flow (or TypeScript syntax error), and this annotation is used to represent the type of this.

For example:

type Foo = {
  bar: number
}

function usesFoo(this: Foo) {
  // Ok, because we've declared `this` should be of type `Foo`
  alert(this.bar)
}

function usesFoo() {
  // Not ok, because this is assumed to be `window` / `self` / `globalThis`,
  // and `.bar` is an invalid property of the global object.
  alert(this.bar)
}

Copy link
Member

Choose a reason for hiding this comment

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

Replying to myself, if the error is Flow-related, it could be because packages such as @babel/plugin-syntax-flow are being downgraded with this PR, and the older version maybe didn't have support for these this annotations.

// Rotate encryption keys every 2**32 messages
// This is just a precaution for a birthday attack, which reduces the
// odds of a collision due to salt reuse to under 10**-18.
Expand Down
Loading