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

bug: capacitor.config.ts cannot support top-level await #6961

Open
oliveryasuna opened this issue Oct 6, 2023 · 8 comments
Open

bug: capacitor.config.ts cannot support top-level await #6961

oliveryasuna opened this issue Oct 6, 2023 · 8 comments
Labels
cli type: feature request A new feature, enhancement, or improvement

Comments

@oliveryasuna
Copy link

oliveryasuna commented Oct 6, 2023

Bug Report

Capacitor Version

💊   Capacitor Doctor  💊 

Latest Dependencies:

  @capacitor/cli: 5.4.2
  @capacitor/core: 5.4.2
  @capacitor/android: 5.4.2
  @capacitor/ios: 5.4.2

Installed Dependencies:

  @capacitor/cli: 5.4.2
  @capacitor/core: 5.4.2
  @capacitor/android: 5.4.2
  @capacitor/ios: 5.4.2

[success] iOS looking great! 👌

Current Behavior

I was not sure if I should report this as a bug or feature. I consider it a bug, but a fix would most likely introduce a new feature.

See the Code Reproduction. It is impossible to use top-level await statements in capacitor.config.ts. This is due to https://github.com/ionic-team/capacitor/blob/main/cli/src/util/node.ts#L12-L50. The module is always loaded as a CommonJS module, which does not support top-level await statements.

You may think that you could make the whole module asynchronous:

// capacitor.config.ts

export default new Promise((resolve, reject) => {
  // ...
});

but Capacitor does not handle this.

Expected Behavior

Capacitor should not limit developers to write a CommonJS-compliant capacitor.config.ts. It is a mobile framework, not a JS one. Thus, in my opinion, it can be mobile-opinionated, but not JS opinionated.

I propose two solutions:

  1. Instead of the DIY compiler found on https://github.com/ionic-team/capacitor/blob/main/cli/src/util/node.ts#L12-L50, Capacitor could use a configuration loader. https://github.com/cosmiconfig/cosmiconfig would be a top choice. This would also enable developers to write their Capacitor configurations in whatever format they so chose.
  2. Somehow allow the developer to specify the module type when importing capacitor.config.ts. Perhaps use a predefined TSConfig file, such as tsconfig.capacitor.json. Better if the code allows developers to specify the file, though.

Code Reproduction

https://github.com/oliveryasuna/capacitor-config-bug

Try running cap add ios, for example.

Here's a config that would cause problems.

// capacitor.config.ts

const myFunc = (async(): Promise<void> => {
});

await myFunc();

return {};
@ionitron-bot ionitron-bot bot added the triage label Oct 6, 2023
@jcesarmobile jcesarmobile added the needs reproduction needs reproducible example to illustrate the issue label Oct 6, 2023
@ionitron-bot
Copy link

ionitron-bot bot commented Oct 6, 2023

This issue needs more information before it can be addressed. In particular, the reporter needs to provide a minimal sample app that demonstrates the issue. If no sample app is provided within 15 days, the issue will be closed.
Please see the Contributing Guide for how to create a Sample App.
Thanks! Ionitron 💙

@ionitron-bot ionitron-bot bot removed the triage label Oct 6, 2023
@Ionitron Ionitron added needs reply needs reply from the user and removed needs reply needs reply from the user labels Oct 6, 2023
@jcesarmobile jcesarmobile added the needs reply needs reply from the user label Oct 6, 2023
@Ionitron Ionitron removed the needs reply needs reply from the user label Oct 6, 2023
@oliveryasuna
Copy link
Author

oliveryasuna commented Oct 6, 2023

Looks like this is a duplicate of #6836. However, that issue was closed. My temporary workaround was to write a script that imports my Capacitor TS config and writes capacitor.config.json.

@oliveryasuna

This comment was marked as abuse.

@jcesarmobile jcesarmobile added type: feature request A new feature, enhancement, or improvement cli and removed needs reproduction needs reproducible example to illustrate the issue labels Nov 27, 2023
@oliveryasuna

This comment was marked as abuse.

@bosh-code
Copy link
Contributor

@oliveryasuna If your not already aware, this feature is coming in capacitor 6: #4299

@jcesarmobile
Copy link
Member

it's not, it allows to use async code, but not top level await

@oliveryasuna
Copy link
Author

@jcesarmobile Still, it will remove the need for top-level await.

Thanks @bosh-code !

@piotr-cz
Copy link

piotr-cz commented Nov 6, 2024

To fix typescript issues, it's compiler options here https://github.com/ionic-team/capacitor/blob/6.1.2/cli/src/util/node.ts#L27-L33 should be changed:

  {
    "compilerOptions": {
-     "module": "CommonJS",
+     "module": "Node16", // Or greater
-     "moduleResolution": "Node",
+     "moduleResolution": "Node16", // Or greater
      "esModuleInterop": true,
      "strict": true,
      "target": "ES2017"
    }
  }

This resolves VSCode issue when I created capacitor.config.ts file with above options and "include: ["capacitor.config.ts"']:

Top-level 'await' expressions are only allowed when the 'module' option is set to 'es2022', 'esnext', 'system', 'node16', 'nodenext', or 'preserve', and the 'target' option is set to 'es2017' or higher.

but I'm not sure it actually works.

Another thing is that since Capacitor v6 requires Node >= 18 compiler options can be updated to this: https://github.com/tsconfig/bases/blob/main/bases/node18.json


Update: above doesn't help in using top level await, probably because capacitor.config.ts is not interpreted as a module.
But helps in importing packages as modules inside async function:

async function generateConfig(): Promise<CapacitorConfig> {
  const { loadEnv } = await import('vite');

  // ...
};

I have tried changing:

  compilerOptions: {
-   module: ts.ModuleKind.CommonJS,
+   module: ts.ModuleKind.Node16,
-   moduleResolution: ts.ModuleResolutionKind.NodeJs,
+   moduleResolution: ts.ModuleResolutionKind.Node16,
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli type: feature request A new feature, enhancement, or improvement
Projects
None yet
Development

No branches or pull requests

5 participants