fix(gatsby): be less aggressive when marking builtin methods as unsafe #30216
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
gatsbot
bot
added
the
status: triage needed
pieh
added
topic: build
vladar
approved these changes
Mar 12, 2021
There was a problem hiding this comment.
Looks good, thanks 👍 I wonder though if this approach is viable in the long run - will we keep adding all possible exceptions? Maybe we can just allow users to opt-out of those warnings somehow?
Not blocking this PR obviously but something to think about. Left one nit but also not blocking.
packages/gatsby/cache-dir/ssr-builtin-trackers/tracking-unsafe-module-wrapper.js
Show resolved
Hide resolved
That is TBD, for me ideally users are covered in a lot of common cases (our defaults), but will think about ignoring those warnings, or ignoring some of them because they should be treated as safe (configurable by users) |
wardpeet
pushed a commit
that referenced
this pull request
Mar 17, 2021
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.
Description
This PR looks to make tracking of unsafe builtins a bit more relaxed - few cases that we gathered so far:
http(s).Agentitself doesn't have sideeffects, so just preparing it without making request should be safe (relevant when usingalgoliasearchpackage as an example)node-gyp-buildused to load some binaries should be safe to ignore (relevant when usingsubscriptions-transport-wsas an example)[ch26858]