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

Dont change global promise props #1294

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dfahlander
Copy link
Collaborator

This PR fails for 2 tests representing apps that imports Promise from a promise polyfill instead of just using the global Promise.

This is PR is based on #1293 but goes a step further.

Many apps and libs are still importing Promise from a polyfill to make them work on IE11. If these apps are doing Promise.resolve() or similar using the imported Promise constructor, we will not be able to track the ongoing transaction or liveQuery context if this commit is released.

Those apps will need to drag down the promise polyfill in a plain script tag instead of importing from the polyfill. The webpacked code must use the global Promise and not an imported one at any time within Dexie transactions and liveQuery querier functions.

This commit should be released in a new major version of Dexie with clear instrutions of how to migrate Promise polyfill dependent code.

We could also make an intermediate fix that is backward compliant but warns in console if they are using import polyfill-pattern (not part of this commit).

The upsides with this commit are:

  • Optimization: We only switch self.Promise between zones.
  • Simplicity: Less code
  • Unobtrusiveness

@dfahlander dfahlander marked this pull request as draft April 29, 2021 11:57
@dfahlander dfahlander changed the title Dont change global promise props2 Dont change global promise props Apr 29, 2021
Copy link

@frikke frikke left a comment

Choose a reason for hiding this comment

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

@dfahlander dfahlander force-pushed the dont-change-global-promise-props2 branch from b3a5596 to 90fcc6f Compare December 11, 2023 15:57
This commit fails for 2 tests representing apps that imports Promise from a promise polyfill instead of just using the global Promise.

Many apps and libs are still importing Promise from a polyfill to make them work on IE11. If these apps are doing Promise.resolve() or similar using the imported Promise constructor, we will not be able to track the ongoing transaction or liveQuery context if this commit is released.

Those apps will need to drag down the promise polyfill in a plain script tag instead of importing from the polyfill. The webpacked code must use the global Promise and not an imported one at any time within Dexie transactions and liveQuery querier functions.

This commit should be released in a new major version of Dexie with clear instrutions of how to migrate Promise polyfill dependent code.

We could also make an intermediate fix that is backward compliant but warns in console if they are using import polyfill-pattern (not part of this commit).

The upsides with this commit are:
* Optimization: We only switch self.Promise between zones.
* Simplicity: Less code
* Unobtrusiveness
@dfahlander dfahlander force-pushed the dont-change-global-promise-props2 branch from 67ca1f0 to 2de7913 Compare December 11, 2023 16:04
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.

2 participants