Fix source maps and simplify getApolloContext to avoid global symbols. #7371
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
I was surprised and frustrated to find that tsc does not honor the --inlineSources compiler option unless source maps are inlined into the generated source files (the --inlineSourceMap option). Since we definitely do not want to inline source maps into source files, we have to use a script to add the sourcesContent property to the .js.map files generated by tsc. Fortunately, Rollup gets this right when generating .cjs.js.map files. Partially fixes #7370.
Fixes the Symbol-related errors reported in #7370.
This seems to be what some versions of TypeScript prefer.
jcreighton
approved these changes
Nov 24, 2020
benjamn
changed the title
benjamn
deleted the
7370-fix-source-maps-and-simplify-getApolloContext
branch
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The problems with source maps reported by @Billy- in #7370 likely snuck in when we switched back from Rollup to
tscfor compiling TypeScript to ESM in #6694.Thanks to @Billy-'s reproduction, I was able to simplify
getApolloContextto avoid relying on globalSymbolproperties. However,React.createContextis still undefined in that module, because webpack resolvesreactto@apollo/client/react/index.js, which suggests to me that module resolution must be misconfigured in that application. This is not a problem that Apollo Client can solve, though this PR may help somewhat.Finally, webpack and/or
ts-loaderseem to preferimport * as React from 'react'overimport React from 'react', and the generated code looks fine to me (both CJS and ESM), so I updated those imports in a few other places.