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

Add missed reexports of MutationFetchPolicy and RefetchWritePolicy to @apollo/client/core #11332

Merged
merged 5 commits into from
Nov 8, 2023

Conversation

asvishnyakov
Copy link
Contributor

Checklist:

  • If this PR contains changes to the library itself (not necessary for e.g. docs updates), please include a changeset (see CONTRIBUTING.md)
  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

@apollo-cla
Copy link

@asvishnyakov: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

Copy link

netlify bot commented Nov 1, 2023

Deploy Preview for apollo-client-docs ready!

Name Link
🔨 Latest commit 280c176
🔍 Latest deploy log https://app.netlify.com/sites/apollo-client-docs/deploys/654b648574886f00087980eb
😎 Deploy Preview https://deploy-preview-11332--apollo-client-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

changeset-bot bot commented Nov 1, 2023

🦋 Changeset detected

Latest commit: 280c176

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@apollo/client Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@phryneas phryneas left a comment

Choose a reason for hiding this comment

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

Good point - thanks for the PR!

@asvishnyakov
Copy link
Contributor Author

asvishnyakov commented Nov 7, 2023

@phryneas I see

Warning: You have changed the public API signature for this project. Please copy the file ".api-reports/temp/api-report.md" to ".api-reports/api-report.md", or perform a local build (which does this automatically).

and I'm sure I should be able to run build locally, but I'm unable to do it even on fresh main branch checkout. I receive the following error after npm ci && npm run build:

[!] Error: Unexpected token
dist/utilities/globals/index.js (6:26)
4: export { invariant, newInvariantError, InvariantError };
5: export var DEV = globalThis.globalThis.__DEV__ !== false !== false;
6: export { DEV as globalThis.__DEV__ !== false };
                             ^
7: //# sourceMappingURL=index.js.map
Error: Unexpected token
    at error (E:\personal\apollo-client\node_modules\rollup\dist\shared\rollup.js:198:30)
    at Module.error (E:\personal\apollo-client\node_modules\rollup\dist\shared\rollup.js:12560:16)
    at Module.tryParse (E:\personal\apollo-client\node_modules\rollup\dist\shared\rollup.js:12937:25)
    at Module.setSource (E:\personal\apollo-client\node_modules\rollup\dist\shared\rollup.js:12842:24)
    at ModuleLoader.addModuleSource (E:\personal\apollo-client\node_modules\rollup\dist\shared\rollup.js:22090:20)

I use Node.js 18.18.2 and npm 9.8.1. May you help me, please?

@phryneas
Copy link
Member

phryneas commented Nov 7, 2023

Yeah, I'm sorry, that message is a bit misleading - please do npm run build && npm run extract-api.

Edit: I'm sorry, I just wanted to give you a quick response - I haven't seen the error you have here before. Are you sure all dependencies are installed in their most recent version?

@asvishnyakov
Copy link
Contributor Author

I just checked out code and run npm ci && npm run build. I don't see any additional requirements in documentation. Does I miss something?

@phryneas
Copy link
Member

phryneas commented Nov 8, 2023

I mean, npm run ci is for the CI - honestly, I've never executed that myself on localhost.

I'd do

npm install
npm run build
npm run extract-api

and I've never seen the error you are seeing here :/

I honestly would like to just fix this up for you, but for some reason I cannot checkout your branch. Do you have the "maintainers are allowed to push onto the PR branch" enabled?

@asvishnyakov
Copy link
Contributor Author

asvishnyakov commented Nov 8, 2023

  1. npm ci is for clean install, and while it's primary usage in CI, it's just mean it will not mutate package-lock.json, remove node_modules, if exists and so on.
    Any way. npm install && npm run build straight after checkout give me exactly the same error.

  2. Thank you! Yes I do
    image

This PR became stranger and stranger 😁

UPD.: Added you as collaborator, just in case

@phryneas
Copy link
Member

phryneas commented Nov 8, 2023

Seems I was running into this problem - I've worked around it now :)

@asvishnyakov
Copy link
Contributor Author

This is a full build log

Log
E:\personal\apollo-client>npm install

@apollo/[email protected] postinstall
patch-package

patch-package 7.0.2
Applying patches...
@testing-library/[email protected]

up to date, audited 960 packages in 6s

150 packages are looking for funding
run npm fund for details

3 vulnerabilities (2 moderate, 1 critical)

To address all issues, run:
npm audit fix

Run npm audit for details.

E:\personal\apollo-client>npm run build

@apollo/[email protected] prebuild
npm run clean

@apollo/[email protected] clean
rimraf dist coverage lib temp

@apollo/[email protected] build
tsc

@apollo/[email protected] postbuild
npm run update-version && npm run invariants && npm run sourcemaps && npm run rollup && npm run prepdist && npm run postprocess-dist && npm run verify-version

@apollo/[email protected] update-version
node config/version.js update

ok

@apollo/[email protected] invariants
ts-node-script config/processInvariants.ts

@apollo/[email protected] sourcemaps
ts-node-script config/rewriteSourceMaps.ts

Rewrote 199 source maps in 144ms

@apollo/[email protected] rollup
rollup -c ./config/rollup.config.js

dist/index.js → dist/main.cjs...
created dist/main.cjs in 32ms

dist\cache/index.js → dist\cache/cache.cjs...
created dist\cache/cache.cjs in 662ms

dist\core/index.js → dist\core/core.cjs...
Risky cross-entry-point nested import of ../cache/inmemory/types.js in core
Risky cross-entry-point nested import of ../version.js in core/ApolloClient.js
Risky cross-entry-point nested import of ../utilities/common/incrementalResult.js in core/QueryManager.js
created dist\core/core.cjs in 474ms

dist\dev/index.js → dist\dev/dev.cjs...
Risky cross-entry-point nested import of ../utilities/globals/invariantWrappers.js in dev/loadErrorMessageHandler.js
Risky cross-entry-point nested import of ../invariantErrorCodes.js in dev/loadDevMessages.js
Risky cross-entry-point nested import of ../invariantErrorCodes.js in dev/loadErrorMessages.js
Risky cross-entry-point nested import of ../../version.js in utilities/globals/invariantWrappers.js
Risky cross-entry-point nested import of ../common/stringifyForDisplay.js in utilities/globals/invariantWrappers.js
created dist\dev/dev.cjs in 135ms

dist\errors/index.js → dist\errors/errors.cjs...
created dist\errors/errors.cjs in 18ms

dist\link\batch/index.js → dist\link\batch/batch.cjs...
created dist\link\batch/batch.cjs in 49ms

dist\link\batch-http/index.js → dist\link\batch-http/batch-http.cjs...
Risky cross-entry-point nested import of ../utils/filterOperationVariables.js in link/batch-http/batchHttpLink.js
created dist\link\batch-http/batch-http.cjs in 51ms

dist\link\context/index.js → dist\link\context/context.cjs...
created dist\link\context/context.cjs in 21ms

dist\link\core/index.js → dist\link\core/core.cjs...
created dist\link\core/core.cjs in 77ms

dist\link\error/index.js → dist\link\error/error.cjs...
created dist\link\error/error.cjs in 21ms

dist\link\http/index.js → dist\link\http/http.cjs...
Risky cross-entry-point nested import of ../../utilities/common/incrementalResult.js in link/http/parseAndCheckHttpResponse.js
created dist\link\http/http.cjs in 178ms

dist\link\persisted-queries/index.js → dist\link\persisted-queries/persisted-queries.cjs...
created dist\link\persisted-queries/persisted-queries.cjs in 27ms

dist\link\retry/index.js → dist\link\retry/retry.cjs...
created dist\link\retry/retry.cjs in 47ms

dist\link\remove-typename/index.js → dist\link\remove-typename/remove-typename.cjs...
created dist\link\remove-typename/remove-typename.cjs in 29ms

dist\link\schema/index.js → dist\link\schema/schema.cjs...
created dist\link\schema/schema.cjs in 20ms

dist\link\subscriptions/index.js → dist\link\subscriptions/subscriptions.cjs...
created dist\link\subscriptions/subscriptions.cjs in 20ms

dist\link\utils/index.js → dist\link\utils/utils.cjs...
created dist\link\utils/utils.cjs in 48ms

dist\link\ws/index.js → dist\link\ws/ws.cjs...
created dist\link\ws/ws.cjs in 18ms

dist\react/index.js → dist\react/react.cjs...
created dist\react/react.cjs in 19ms

dist\react\components/index.js → dist\react\components/components.cjs...
created dist\react\components/components.cjs in 47ms

dist\react\context/index.js → dist\react\context/context.cjs...
created dist\react\context/context.cjs in 39ms

dist\react\hoc/index.js → dist\react\hoc/hoc.cjs...
created dist\react\hoc/hoc.cjs in 100ms

dist\react\hooks/index.js → dist\react\hooks/hooks.cjs...
Risky cross-entry-point nested import of ../cache/index.js in react/hooks/useSuspenseQuery.js
Risky cross-entry-point nested import of ../cache/QueryReference.js in react/hooks/useBackgroundQuery.js
Risky cross-entry-point nested import of ../cache/index.js in react/hooks/useBackgroundQuery.js
Risky cross-entry-point nested import of ../cache/QueryReference.js in react/hooks/useReadQuery.js
created dist\react\hooks/hooks.cjs in 270ms

dist\react\parser/index.js → dist\react\parser/parser.cjs...
created dist\react\parser/parser.cjs in 19ms

dist\react\ssr/index.js → dist\react\ssr/ssr.cjs...
created dist\react\ssr/ssr.cjs in 44ms

dist\testing/index.js → dist\testing/testing.cjs...
created dist\testing/testing.cjs in 23ms

dist\testing\core/index.js → dist\testing\core/core.cjs...
created dist\testing\core/core.cjs in 73ms

dist\utilities/index.js → dist\utilities/utilities.cjs...
created dist\utilities/utilities.cjs in 270ms

dist\utilities\globals/index.js → dist\utilities\globals/globals.cjs...
[!] Error: Unexpected token
dist/utilities/globals/index.js (6:26)
4: export { invariant, newInvariantError, InvariantError };
5: export var DEV = globalThis.globalThis.DEV !== false !== false;
6: export { DEV as globalThis.DEV !== false };
^
7: //# sourceMappingURL=index.js.map
Error: Unexpected token
at error (E:\personal\apollo-client\node_modules\rollup\dist\shared\rollup.js:198:30)
at Module.error (E:\personal\apollo-client\node_modules\rollup\dist\shared\rollup.js:12560:16)
at Module.tryParse (E:\personal\apollo-client\node_modules\rollup\dist\shared\rollup.js:12937:25)
at Module.setSource (E:\personal\apollo-client\node_modules\rollup\dist\shared\rollup.js:12842:24)
at ModuleLoader.addModuleSource (E:\personal\apollo-client\node_modules\rollup\dist\shared\rollup.js:22090:20)

I would appreciate if you will share yours, so may be I will be able to fix the issue, I wanted to do a few more PRs later.

@phryneas
Copy link
Member

phryneas commented Nov 8, 2023

Ah, you're using Windows. That might be the reason - we don't get that a lot.

if (
!["utilities/globals/index.js", "config/jest/setup.js"].includes(
relativeFilePath
)
)

These lines are probably your problem, as on your OS those paths might look differently :/

Could you try to use a path.join there and see if that helps?

@phryneas phryneas merged commit 291aea5 into apollographql:main Nov 8, 2023
9 checks passed
@github-actions github-actions bot mentioned this pull request Nov 8, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants