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: vue, remove commonjs modules now that ionic core is all es modules #25104

Closed
4 of 6 tasks
kaischo opened this issue Apr 12, 2022 · 14 comments
Closed
4 of 6 tasks

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

kaischo opened this issue Apr 12, 2022 · 14 comments
Labels
package: react @ionic/react package package: vue @ionic/vue package type: bug a confirmed bug report

Comments

@kaischo
Copy link

kaischo commented Apr 12, 2022

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x

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
Copy link
Contributor

liamdebeasi commented Apr 15, 2022

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.

@liamdebeasi liamdebeasi self-assigned this Apr 15, 2022
@liamdebeasi liamdebeasi changed the title bug: tests fail to run when using vitest bug: vue, remove commonjs modules now that ionic core is all es modules Apr 19, 2022
@liamdebeasi liamdebeasi added package: vue @ionic/vue package type: bug a confirmed bug report labels Apr 19, 2022
@ionitron-bot ionitron-bot bot removed the triage label Apr 19, 2022
@liamdebeasi liamdebeasi added triage package: react @ionic/react package labels Apr 19, 2022
@ionitron-bot ionitron-bot bot removed the triage label Apr 19, 2022
@liamdebeasi liamdebeasi removed their assignment Apr 19, 2022
@crobinson-expl
Copy link

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
Copy link

ghost commented May 8, 2022

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

@riderx
Copy link

riderx commented Jun 8, 2022

@crobinson-expl @jrobeson i think that related to the bug i got here : #24800
i don't understand why commonjs are coming to mess it all.

@riderx
Copy link

riderx commented Jun 8, 2022

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
Copy link

ghost commented Jun 8, 2022

@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
Copy link

riderx commented Jun 11, 2022

@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
Copy link

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
Copy link

SimonGolms commented Jun 20, 2022

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: vitest-dev/vitest#1452 (comment)

/// <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

This comment was marked as off-topic.

@richierockskool

This comment was marked as off-topic.

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Sep 29, 2022

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 added a commit that referenced this issue Sep 30, 2022
liamdebeasi added a commit that referenced this issue Sep 30, 2022
resolves #25104

BREAKING CHANGE:

`@ionic/vue` and `@ionic/vue-router` no longer ship a CommonJS entry point. Instead, only an ES Module entry point is provided for improved compatibility with Vite.
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #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
Copy link

ionitron-bot bot commented Oct 30, 2022

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.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: react @ionic/react package package: vue @ionic/vue package type: bug a confirmed bug report
Projects
None yet
Development

No branches or pull requests

7 participants