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

Invalid ESM package of allure-js-commons when using allure-cypress #1136

Open
1 task done
nbouvrette opened this issue Sep 8, 2024 · 7 comments
Open
1 task done
Labels
theme:cypress type:bug Something isn't working

Comments

@nbouvrette
Copy link

nbouvrette commented Sep 8, 2024

Describe the Bug

I am using Cypress and trying to use the allure-cypress package. I noticed some odd issues using this package in Cypress to make it work with TypeScript and now when I try to run a test I Module not found not found errors.

Steps to Reproduce

  1. You configure your Cypress to use TypeScript with the following configuration:
{
  "compilerOptions": {
    "baseUrl": ".",
    "lib": ["es5", "dom", "dom.iterable", "esnext"],
    "types": ["cypress", "node"],
    "esModuleInterop": true,
    "isolatedModules": true,
    "declaration": true,
    "strict": true,
    "moduleResolution": "bundler",
    "module": "ESNext"
  },
  "include": ["**/*.ts"]
}
  1. I have never had to use moduleResolution": "bundler" in the past and I am not sure why this seemed like the only way to make the package work. I suspect because of this its also causing the following issue.

  2. When I open a test that, that relies on another ESM package, I get the following error, even if I don't use Allure in that test. The only place it is loaded is in the Config where allureCypress is called.

  3. I get a bunch of errors like this:

Error: Webpack Compilation Error
Module not found: Error: Can't resolve '../model' in '/my-project/node_modules/allure-js-commons/dist/esm/src/new/framework'
Did you mean 'model.js'?
BREAKING CHANGE: The request '../model' failed to resolve only because it was resolved as fully specified
(probably because the origin is strict EcmaScript Module, e. g. a module with javascript mimetype, a '*.mjs' file, or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.

Module not found: Error: Can't resolve './AllureResults' in '/my-project/node_modules/allure-js-commons/dist/esm/src/new/framework'
Did you mean 'AllureResults.js'?
BREAKING CHANGE: The request './AllureResults' failed to resolve only because it was resolved as fully specified
(probably because the origin is strict EcmaScript Module, e. g. a module with javascript mimetype, a '*.mjs' file, or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.
  1. When I inspect the allure-js-commons esm files - I see several issues such as:
  • node_modules/allure-js-commons/dist/esm/src/new/framework/AllureCommandStep.js: import { LabelName, LinkType, Stage, Status, } from "../model"; this is not valid esm and should be import { LabelName, LinkType, Stage, Status, } from "../model.js";
  • node_modules/allure-js-commons/dist/esm/src/new/framework/AllureExecutable.js: import { stepResult } from "./AllureResults"; this is not valid esm and should be import { stepResult } from "./AllureResults.js";

The solution is to compile the ESM version of the package correctly to fix the errors. I have a build script that does just that if you are not sure how to fix the extensions (this is the simplest solution I found on my packages): https://github.com/properties-file/properties-file/blob/03a795b57ebe92b93e1869cdade351b54301ab2c/src/build-scripts/build.ts#L78

In addition to that, I am hoping it will resolve the first issue (moduleResolution": "bundler") so that we don't have to use the ESM version since by default Cypress uses CommonJS libraries. I think we should try to respect this behavior and only use ESM when a package is not available in CommonJS, which is not the case for your packages.

Expected Behaviour

I would expect my Cypress test to not crash when using Allure and TypeScript.

Screenshots or Additional Context

No response

What Language are you using?

TypeScript

What Framework/Allure Integration you are using?

allure-cypress

What version of Allure Integration you are using?

2.15.1

What version of Allure Report you are using?

2.15.1

Code of Conduct

  • I agree to follow this project's Code of Conduct
@nbouvrette nbouvrette added the type:bug Something isn't working label Sep 8, 2024
@nzin-alayacare
Copy link

nzin-alayacare commented Sep 11, 2024

Hi,
I dont know if @davitperaze , @baev or @pfyod you can help us to fix that?

@baev baev transferred this issue from allure-framework/allure2 Sep 12, 2024
@baev
Copy link
Member

baev commented Sep 12, 2024

Please update to allure-cypress@3.0.0-beta.12

Not many changes are planned to be added before the 3.0.0 release, so it should be safe to use it.

@nzin-alayacare
Copy link

thanks, we will test that!

@nbouvrette
Copy link
Author

Thanks @baev - I just gabe a quick test to the new 3.0.0 allure-cypress release and I am getting this error:

Cannot find module 'allure-cypress/reporter'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?ts(2792)

When doing this:

import { allureCypress } from 'allure-cypress/reporter'` from the `cypress.config.ts` file.

I took a quick look and I suspect there is an issue with your package.json file. Instead of doing this:

  "exports": {
    ".": {
      "import": "./dist/esm/index.js",
      "require": "./dist/cjs/index.js",
      "types": "./dist/types/index.d.ts"
    },
    "./reporter": {
      "import": "./dist/esm/reporter.js",
      "require": "./dist/cjs/reporter.js",
      "types": "./dist/types/reporter.d.ts"
    }
  },

Could you try doing this instead:

  "exports": {
    ".": {
      "import": {
        "types": "./dist/types/index.d.ts",
        "default": "./dist/esm/index.js"
      },
      "require": {
        "types": "./dist/types/index.d.ts",
        "default": "./dist/cjs/index.js"
      }
    },
    "./reporter": {
      "import": {
        "types": "./dist/types/reporter.d.ts",
        "default": "./dist/esm/reporter.js"
      },
      "require": {
        "types": "./dist/types/reporter.d.ts",
        "default": "./dist/cjs/reporter.js"
    }
  },

I did a quick spot check and it looks like the ESM issues mentionned in my previous comment were fixed but the files are so different I might not have caught all cases. I would suggest to try to the fix issue mentionned here first - we should not need ESM by default on your package.

@nbouvrette
Copy link
Author

Actually, I don't think my previous message will help - I just noticed that in your 3.0.0 package you don't have any cjs directory under dist - most likely you have an issue with your build process.

@baev please try to fix this issue so that we do not have to force using ESM with Cypresss

@delatrie
Copy link
Collaborator

Hi, @nbouvrette !

Not sure that's the case. npm pack allure-cypress@3.0.0 shows everything is in place:

> npm pack allure-cypress@3.0.0
npm notice
npm notice package: allure-cypress@3.0.0
npm notice === Tarball Contents ===
npm notice 8.9kB  README.md
npm notice 18.0kB dist/cjs/index.js
npm notice 86.1kB dist/cjs/index.js.map
npm notice 399B   dist/cjs/model.js
npm notice 5.0kB  dist/cjs/model.js.map
npm notice 24B    dist/cjs/package.json
npm notice 25.7kB dist/cjs/reporter.js
npm notice 42.1kB dist/cjs/reporter.js.map
npm notice 22.9kB dist/cjs/runtime.js
npm notice 44.3kB dist/cjs/runtime.js.map
npm notice 2.1kB  dist/cjs/state.js
npm notice 3.6kB  dist/cjs/state.js.map
npm notice 8.2kB  dist/cjs/utils.js
npm notice 16.1kB dist/cjs/utils.js.map
npm notice 17.5kB dist/esm/index.js
npm notice 85.2kB dist/esm/index.js.map
npm notice 179B   dist/esm/model.js
npm notice 4.9kB  dist/esm/model.js.map
npm notice 22B    dist/esm/package.json
npm notice 25.3kB dist/esm/reporter.js
npm notice 39.0kB dist/esm/reporter.js.map
npm notice 20.9kB dist/esm/runtime.js
npm notice 41.3kB dist/esm/runtime.js.map
npm notice 1.2kB  dist/esm/state.js
npm notice 3.2kB  dist/esm/state.js.map
npm notice 7.0kB  dist/esm/utils.js
npm notice 13.8kB dist/esm/utils.js.map
npm notice 35B    dist/types/index.d.ts
npm notice 4.6kB  dist/types/model.d.ts
npm notice 3.6kB  dist/types/reporter.d.ts
npm notice 3.3kB  dist/types/runtime.d.ts
npm notice 971B   dist/types/state.d.ts
npm notice 2.2kB  dist/types/utils.d.ts
npm notice 3.4kB  package.json
npm notice === Tarball Details ===
npm notice name:          allure-cypress
npm notice version:       3.0.0
npm notice filename:      allure-cypress-3.0.0.tgz
npm notice package size:  138.7 kB
npm notice unpacked size: 561.0 kB
npm notice shasum:        5105b9aa2a50e01ecb2430cbd57d06ab0572d37e
npm notice integrity:     sha512-mBvA9b9bbgDLC[...]YrTmU2WTjuEfA==
npm notice total files:   34
npm notice

Could you please take a look at this repo? It runs Cypress with typescript, and the result files are generated just fine (you may check the action). Cypress uses CJS there for sure. You may validate that by playing with ./node_modules/allure-cypress/dist/cjs and watching the side effects (or removing the esm dir).

Does that work for you? If not, could you please show the difference in the setups?

@nbouvrette
Copy link
Author

nbouvrette commented Sep 18, 2024

@delatrie you're right - not sure what happened last night when I was checking this - probably it was getting too late for me and I got confused.

Having said this, the Cannot find module 'allure-cypress/reporter'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?ts(2792) error is still a problem.

I checked your example repo and you are using:

    "moduleResolution": "NodeNext",
    "module": "NodeNext"

In your tsconfig.json - the problem with this is that now everyone using your package will have this problem:

Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../support/utils.js'?ts(2835)

When using simple imports like this:

import {
  isNonNullObject,
  isNotEmptyString,
  isObjectWithAttribute,
  isValidUrl,
  WithUnknown,
} from '../support/utils'

This would make TypeScript projects look very odd, importing .js fils everywhere. What I would recommend is finding a way to build your package so that is supports this "standard" Cypress tsconfig.json:

  "compilerOptions": {
    "target": "ESNext",
    "skipLibCheck": true,
    "lib": ["esnext", "dom", "dom.iterable"],
    "types": ["cypress", "node"],
    "module": "ESNext",
    "declaration": true,
    "downlevelIteration": true,
    "strict": true
  },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme:cypress type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants