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

[SDK 50][expo-web] @apollo-client fails during expo export with metro for web #25965

Closed
mojavad opened this issue Dec 15, 2023 · 14 comments
Closed
Assignees
Labels

Comments

@mojavad
Copy link
Contributor

mojavad commented Dec 15, 2023

Minimal reproducible example

https://github.com/mojavad/apollo-sdk50-bug-repro

Summary

When exporting with metro for web with the new SDK 50 beta, we are facing bundling issues for production builds on web as we use @apollo/client.

The error we get is the following:

Error: node_modules/@apollo/client/utilities/globals/index.js: Property exported of ExportSpecifier expected node to be of a type ["Identifier","StringLiteral"] but instead got "BooleanLiteral"

This error doesn't appear when exporting for ios with version SDK 50.

Additionally, this error doesn't show for expo start, which could be due to minification with Babel?

Environment

  expo-env-info 1.0.5 environment info:
    System:
      OS: macOS 14.2
      Shell: 5.9 - /bin/zsh
    Binaries:
      Node: 18.17.1 - ~/.nvm/versions/node/v18.17.1/bin/node
      Yarn: 1.22.19 - ~/.yarn/bin/yarn
      npm: 9.6.7 - ~/.nvm/versions/node/v18.17.1/bin/npm
      Watchman: 2023.08.28.00 - /opt/homebrew/bin/watchman
    Managers:
      CocoaPods: 1.12.1 - /opt/homebrew/bin/pod
    SDKs:
      iOS SDK:
        Platforms: DriverKit 23.0, iOS 17.0, macOS 14.0, tvOS 17.0, visionOS 1.0, watchOS 10.0
    IDEs:
      Android Studio: 2022.1 AI-221.6008.13.2211.9619390
      Xcode: 15.0/15A5209g - /usr/bin/xcodebuild
    npmPackages:
      expo: ~50.0.0-preview.3 => 50.0.0-preview.3 
      react: 18.2.0 => 18.2.0 
      react-dom: 18.2.0 => 18.2.0 
      react-native: 0.73.0 => 0.73.0 
      react-native-web: ~0.19.6 => 0.19.9 
    npmGlobalPackages:
      eas-cli: 5.5.0
    Expo Workflow: managed
@mojavad mojavad added the needs validation Issue needs to be validated label Dec 15, 2023
@expo-bot expo-bot added needs review Issue is ready to be reviewed by a maintainer and removed needs validation Issue needs to be validated labels Dec 15, 2023
@expo-bot
Copy link
Collaborator

Thank you for filing this issue!
This comment acknowledges we believe this may be a bug and there’s enough information to investigate it.
However, we can’t promise any sort of timeline for resolution. We prioritize issues based on severity, breadth of impact, and alignment with our roadmap. If you’d like to help move it more quickly, you can continue to investigate it more deeply and/or you can open a pull request that fixes the cause.

@github-actions github-actions bot removed the needs review Issue is ready to be reviewed by a maintainer label Dec 18, 2023
@marklawlor
Copy link
Contributor

@marklawlor marklawlor added the Router expo-router label Dec 18, 2023
@umairb-pedal
Copy link

umairb-pedal commented Dec 28, 2023

@EvanBacon Issue appears to stem from this line https://github.com/apollographql/apollo-client/blob/f92db4e809e113000b30fc081b7cf1648efe3446/src/utilities/globals/index.ts#L18

It's actually line 19 that is causing the problem: The export of __DEV__. Looks like the react native babel plugin doesn't like the export for some reason. Conflicts with the built in __DEV__?

@keeganthomp
Copy link

Any resolution to this?

@keeganthomp
Copy link

This is really gross 😅 but a stop gap is adding the below function to a postinstall script. Will DEFINATELY want to revisit this if you go this route in the interim :)

const fs = require('fs');

const fixVarConflict = () => {
  const pathToProblematicFile =
    'node_modules/@apollo/client/utilities/globals/index.js';
  // check if file exists
  const fileExists = fs.existsSync(pathToProblematicFile);
  if (!fileExists) {
    console.log('module not found');
    return;
  }
  const problematicFile = fs.readFileSync(pathToProblematicFile, 'utf8');
  const fixedFile = problematicFile.replace(
    'export { DEV as __DEV__ };',
    '// export { DEV as __DEV__ };',
  );
  fs.writeFileSync(pathToProblematicFile, fixedFile);
};

fixVarConflict()

in package.json, something like:

...
"postinstall": "node ./scripts/fix-web-build.js",

@umairb-pedal
Copy link

This is really gross 😅 but a stop gap is adding the below function to a postinstall script. Will DEFINATELY want to revisit this if you go this route in the interim :)

const fs = require('fs');

const fixVarConflict = () => {
  const pathToProblematicFile =
    'node_modules/@apollo/client/utilities/globals/index.js';
  // check if file exists
  const fileExists = fs.existsSync(pathToProblematicFile);
  if (!fileExists) {
    console.log('module not found');
    return;
  }
  const problematicFile = fs.readFileSync(pathToProblematicFile, 'utf8');
  const fixedFile = problematicFile.replace(
    'export { DEV as __DEV__ };',
    '// export { DEV as __DEV__ };',
  );
  fs.writeFileSync(pathToProblematicFile, fixedFile);
};

fixVarConflict()

in package.json, something like:

...
"postinstall": "node ./scripts/fix-web-build.js",

I recommend the following package: https://www.npmjs.com/package/patch-package. Utilizes git patch to update the code, saves you from writing a custom script file.

(if you're using yarn, upgrade to yarn 4 and that has built in support for patches - no need for additional packages).

facebook-github-bot pushed a commit to facebook/metro that referenced this issue Jan 22, 2024
…#1195)

Summary:
We've encountered this as part of a report on the `expo` repo: expo/expo#25965

There, an issue is caused by a dependency using an export alias named `__DEV__`, which the `inlinePlugin` is trying to replace:

```js
export { DEV as __DEV__ };
```

This only occurs in certain cases, since usually, as part of the iOS/Android builds, we would be applying the `inlinePlugin` only after export statements, where this issue may occur, have been transpiled away.

However, looking at this issue, I found more cases that would fail with the current implementation of the `inlinePlugin`. This is part of what I added in tests but not an exhaustive list:
- Shorthand object methods (`{ __DEV__() {} }`)
- Labelled statements (`__DEV__: {};`)
- Class methods (`class { __DEV__() {} }`)
- Optional property member access (`x?.__DEV__`)

All of these issues can be addressed by letting this replacement use `ReferencedIdentifier` as a visitor, rather than just `Identifier`.

Pull Request resolved: #1195

Test Plan: - Tests have been added to `inline-plugin-test.js` to reflect the mentioned cases.

Reviewed By: huntie

Differential Revision: D52909519

Pulled By: motiz88

fbshipit-source-id: 37a6459bc917701fe8474c33ccae41cb484e92f0
robhogan pushed a commit to facebook/metro that referenced this issue Jan 29, 2024
…#1195)

Summary:
We've encountered this as part of a report on the `expo` repo: expo/expo#25965

There, an issue is caused by a dependency using an export alias named `__DEV__`, which the `inlinePlugin` is trying to replace:

```js
export { DEV as __DEV__ };
```

This only occurs in certain cases, since usually, as part of the iOS/Android builds, we would be applying the `inlinePlugin` only after export statements, where this issue may occur, have been transpiled away.

However, looking at this issue, I found more cases that would fail with the current implementation of the `inlinePlugin`. This is part of what I added in tests but not an exhaustive list:
- Shorthand object methods (`{ __DEV__() {} }`)
- Labelled statements (`__DEV__: {};`)
- Class methods (`class { __DEV__() {} }`)
- Optional property member access (`x?.__DEV__`)

All of these issues can be addressed by letting this replacement use `ReferencedIdentifier` as a visitor, rather than just `Identifier`.

Pull Request resolved: #1195

Test Plan: - Tests have been added to `inline-plugin-test.js` to reflect the mentioned cases.

Reviewed By: huntie

Differential Revision: D52909519

Pulled By: motiz88

fbshipit-source-id: 37a6459bc917701fe8474c33ccae41cb484e92f0
robhogan pushed a commit to facebook/metro that referenced this issue Jan 30, 2024
…#1195)

Summary:
We've encountered this as part of a report on the `expo` repo: expo/expo#25965

There, an issue is caused by a dependency using an export alias named `__DEV__`, which the `inlinePlugin` is trying to replace:

```js
export { DEV as __DEV__ };
```

This only occurs in certain cases, since usually, as part of the iOS/Android builds, we would be applying the `inlinePlugin` only after export statements, where this issue may occur, have been transpiled away.

However, looking at this issue, I found more cases that would fail with the current implementation of the `inlinePlugin`. This is part of what I added in tests but not an exhaustive list:
- Shorthand object methods (`{ __DEV__() {} }`)
- Labelled statements (`__DEV__: {};`)
- Class methods (`class { __DEV__() {} }`)
- Optional property member access (`x?.__DEV__`)

All of these issues can be addressed by letting this replacement use `ReferencedIdentifier` as a visitor, rather than just `Identifier`.

Pull Request resolved: #1195

Test Plan: - Tests have been added to `inline-plugin-test.js` to reflect the mentioned cases.

Reviewed By: huntie

Differential Revision: D52909519

Pulled By: motiz88

fbshipit-source-id: 37a6459bc917701fe8474c33ccae41cb484e92f0
@mojavad
Copy link
Contributor Author

mojavad commented Feb 4, 2024

@EvanBacon saw that a fix was merged into Metro directly. Does this mean this issue can be closed? 🙂

@marklawlor
Copy link
Contributor

Fixed in expo@50.0.5.

@wollerman
Copy link

@marklawlor I've tried running npx expo export --clear --platform web on expo@50.0.5 and expo@50.0.14 with the same failure message.

Is this something explicitly fixed by expo or resolved in metro/react-native? I'm trying to figure out which combination of package updates will hopefully resolve this.

Thanks in advance for any help!

@marklawlor
Copy link
Contributor

@wollerman If you have a failure message on the latest version then you most likely have a different issue. Multiple different issues can have the same error message.

Please create a new issue with a reproduction

@younes0
Copy link

younes0 commented May 11, 2024

@marklawlor @wollerman this happens with expo51, which didn't with expo50.
see the reproduction here: https://github.com/younes0/apollo-sdk51-bug-repro

running npx expo export --clear --platform web results in:

Web Bundling failed 11730ms node_modules/expo-router/entry.js (1197 modules)
error: node_modules/@apollo/client/utilities/globals/index.js: /projects/apollo-sdk51-bug-repro/node_modules/@apollo/client/utilities/globals/index.js: Property exported of ExportSpecifier expected node to be of a type ["Identifier","StringLiteral"] but instead got "BooleanLiteral"
λ Bundled 11807ms node_modules/expo-router/node/render.js (1093 modules)
Error: node_modules/@apollo/client/utilities/globals/index.js: /projects/apollo-sdk51-bug-repro/node_modules/@apollo/client/utilities/globals/index.js: Property exported of ExportSpecifier expected node to be of a type ["Identifier","StringLiteral"] but instead got "BooleanLiteral"
Error: node_modules/@apollo/client/utilities/globals/index.js: /projects/apollo-sdk51-bug-repro/node_modules/@apollo/client/utilities/globals/index.js: Property exported of ExportSpecifier expected node to be of a type ["Identifier","StringLiteral"] but instead got "BooleanLiteral"
    at MetroBundlerDevServer.getStaticResourcesAsync (/projects/apollo-sdk51-bug-repro/node_modules/@expo/cli/src/start/server/metro/MetroBundlerDevServer.ts:325:13)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Promise.all (index 0)
    at exportFromServerAsync (/projects/apollo-sdk51-bug-repro/node_modules/@expo/cli/src/export/exportStaticAsync.ts:196:66)
    at unstable_exportStaticAsync (/projects/apollo-sdk51-bug-repro/node_modules/@expo/cli/src/export/exportStaticAsync.ts:94:12)
    at exportAppAsync (/projects/apollo-sdk51-bug-repro/node_modules/@expo/cli/src/export/exportApp.ts:169:7)
    at exportAsync (/projects/apollo-sdk51-bug-repro/node_modules/@expo/cli/src/export/exportAsync.ts:18:3)

Can we reopen this issue and rename it ?

@ericlyoung
Copy link

Just hit this now after updating to 51:

 => ERROR [8/9] RUN npx expo export -p web                                                     195.1s
------
 > [8/9] RUN npx expo export -p web:
2.210 (node:16) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
2.210 (Use `node --trace-deprecation ...` to show where the warning was created)
4.991 Starting Metro Bundler
194.4 Web Bundling failed 189434ms node_modules/expo-router/entry.js (1661 modules)
194.4
194.4 SyntaxError: node_modules/@apollo/client/utilities/globals/index.js: /app/node_modules/@apollo/client/utilities/globals/index.js: Property exported of ExportSpecifier expected node to be of a type ["Identifi
er","StringLiteral"] but instead got "BooleanLiteral"
194.4 TypeError: /app/node_modules/@apollo/client/utilities/globals/index.js: Property exported of ExportSpecifier expected node to be of a type ["Identifier","StringLiteral"] but instead got "BooleanLiteral"

@marklawlor
Copy link
Contributor

Tracking the SDK51 regression under a new issue #28785

@Herklos
Copy link

Herklos commented May 13, 2024

Fixed with "expo": "~51.0.5" for me. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests