ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.06k stars 13.51k forks source link

bug: vue, remove commonjs modules now that ionic core is all es modules #25104

Closed kaischo closed 2 years ago

kaischo commented 2 years ago

Prerequisites

Ionic Framework Version

Current Behavior

Unit Tests with vitest fail due to the following issue: image

I already added the proposed workaround to the vitest config, but the issue persists.

Expected Behavior

Unit tests with vitest should not fail to run when an ionic component is used in the tested component.

Steps to Reproduce

Simply check out the example project i provided below and run npm i and npm test. The test for the component including an ionic component will fail with the message above.

Code Reproduction URL

https://github.com/kaischo/vitest_ionic_minimal

Ionic Info

Ionic:

Ionic CLI : 6.19.0 (/Users/kai/.nvm/versions/node/v16.14.2/lib/node_modules/@ionic/cli) Ionic Framework : @ionic/vue 6.0.16

Capacitor:

Capacitor CLI : 3.4.3 @capacitor/android : not installed @capacitor/core : 3.4.3 @capacitor/ios : not installed

Utility:

cordova-res : not installed globally native-run : 1.5.0

System:

NodeJS : v16.14.2 (/Users/kai/.nvm/versions/node/v16.14.2/bin/node) npm : 8.5.0 OS : macOS Big Sur

Additional Information

Adding @ionic/core to the inline deps field of the vitest config does not seem to help for some reason. Vitest output suggests the following: Module /Users/kai/Desktop/ionic test/vitest_minimal/node_modules/@ionic/core/components/ion-accordion.js:4 seems to be an ES Module but shipped in a CommonJS package. You might want to create an issue to the package "@ionic/core" asking them to ship the file in .mjs extension or add "type": "module" in their package.json.

I am thankful for any tips how to fix this issue on my own or any workarounds.

liamdebeasi commented 2 years ago

Thanks for the issue. I am still digging into this, but it looks like the @ionic/vue package ships with CommonJS and ES Module files whereas @ionic/core only ships as ES Modules. Vitest/Vite loads the CommonJS version of Ionic Vue, which loads the ES Module version of Ionic Core. As a result, this error occurs.

We should probably get rid of the CommonJS version now that we are using ES Modules. Though it is a bit surprising that Vite/Vitest is not loading the ES Module version of @ionic/vue by default.

edit: We should probably make this change for React as well.

crobinson-expl commented 2 years ago

I was able to work around this issue by adding a vitest.config.ts file with the following contents. The alias for "@ionic/vue" is the important line, which forces vitest to load the ES Modules for that package instead of the default CommonJS ones.

import { fileURLToPath, URL } from "url";

import { defineConfig } from "vite";
import vue from "@vitejs/plugin-vue";

export default defineConfig({
    plugins: [vue()],
    resolve: {
        alias: {
            "@": fileURLToPath(new URL("./src", import.meta.url)),
            "@ionic/vue": fileURLToPath(new URL("./node_modules/@ionic/vue/dist/index.esm", import.meta.url)),
        },
    },
});
ghost commented 2 years ago

but what exactly is causing it to load the commonjs version in the first place?

riderx commented 2 years ago

@crobinson-expl @jrobeson i think that related to the bug i got here : https://github.com/ionic-team/ionic-framework/issues/24800 i don't understand why commonjs are coming to mess it all.

riderx commented 2 years ago

I just found something who fix broken modules in vite : https://github.com/originjs/vite-plugins/tree/main/packages/vite-plugin-commonjs. i tried and than fix my issue on my side, hope that will help you too

ghost commented 2 years ago

@riderx i dont' see why we need to use that plugin though. as far a i can tell, in the case of this issue we should not be seeing commonjs modules at all. The linked issue seems specific to vue, while I'm having this problem with react.

riderx commented 2 years ago

@jrobeson agree we should't need that but some package on the way of going es module get lost ^^ my issue is with vue and vite you use vite or rollup too ?

paulsutherland commented 2 years ago

I had issues using the work around suggested by @crobinson-expl where the css files were not found. My work around to solve this is:

import { fileURLToPath, URL } from 'url'

import { defineConfig } from 'vite'
import vue from '@vitejs/plugin-vue'

export default defineConfig({
  plugins: [vue()],
  resolve: {
    alias: {
      '@': fileURLToPath(new URL('./src', import.meta.url)),
      '@ionic/vue/css': fileURLToPath(new URL('./node_modules/@ionic/vue/css', import.meta.url)),
      '@ionic/vue': fileURLToPath(new URL('./node_modules/@ionic/vue/dist/index.esm.js', import.meta.url))
    },
  },
  test: {
    globals: true,
    environment: 'jsdom',
  },
})
SimonGolms commented 2 years ago

I ended up here due to the same error in react and none of the mentioned workarounds (with react adaptation) worked in my case. However, what solved it was adding the property resolve: { mainFields: ['module'] } to vite.config.ts from this issue: https://github.com/vitest-dev/vitest/issues/1452#issuecomment-1150752644

/// <reference types="vitest" />
/// <reference types="vite/client" />
import react from '@vitejs/plugin-react';
import { defineConfig } from 'vite';

export default defineConfig({
  plugins: [react()],
  resolve: {
    // Workaround to fix inline dependency of a dependency, which is the case in @ionic/react
    mainFields: ['module'],
  },
  test: {
    environment: 'jsdom',
    globals: true,
    setupFiles: './src/setupTests.ts',
  },
});

EDIT: After further attempts and following the way to test ionic-react components with reference to the blog post (Testing Ionic React Apps with Jest and React Testing Library), it fails again with the library @ionic/react-test-utils, which currently cannot resolve the modules furthermore.

richierockskool commented 2 years ago

/usr/local/lib/node_modules/npm/node_modules/isexe/index.js:8 export { isexe } ^^^^^^

SyntaxError: Unexpected token 'export' at Object.compileFunction (node:vm:352:18) at wrapSafe (node:internal/modules/cjs/loader:1033:15) at Module._compile (node:internal/modules/cjs/loader:1069:27) at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10) at Module.load (node:internal/modules/cjs/loader:981:32) at Function.Module._load (node:internal/modules/cjs/loader:822:12) at Module.require (node:internal/modules/cjs/loader:1005:19) at require (node:internal/modules/cjs/helpers:102:18) at Object. (/usr/local/lib/node_modules/npm/node_modules/which/which.js:7:15) at Module._compile (node:internal/modules/cjs/loader:1105:14)

Help I can not get out of this loop! when I try npm install

richierockskool commented 2 years ago

richierockskool / homebridge-inkbird-wifi-gateway

liamdebeasi commented 2 years ago

Hi everyone,

I have a dev build that should resolve this issue in Ionic Vue: 6.2.9-dev.11664471488.1801ff98.

Please note that this is a build of Ionic 7 (we are considering this a breaking change to minimize disruptions to developer workflows). As a result, this dev build is subject to the Ionic 7 Breaking Changes.

We plan on making this change to Ionic React too. I will update this comment when I have a dev build for that.

edit: Here is the build for Ionic React: 6.2.9-dev.11664471667.111b2cca

liamdebeasi commented 2 years ago

Thanks for the issue. This has been resolved via https://github.com/ionic-team/ionic-framework/pull/26054, and a fix will be available in an upcoming major release of Ionic Framework. Please let me know if you run into any issues with the dev build.

ionitron-bot[bot] commented 2 years ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.