Dont change global promise props #1294
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dfahlander
changed the title
frikke
suggested changes
Jun 30, 2023
dfahlander
force-pushed
the
dont-change-global-promise-props2
branch
from
b3a5596
to
90fcc6f
Compare
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
force-pushed
the
dont-change-global-promise-props2
branch
from
67ca1f0
to
2de7913
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: