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

change join import to path default import for mjs compatibility #32

Merged
merged 1 commit into from
Oct 17, 2019

Conversation

alejo90
Copy link
Contributor

@alejo90 alejo90 commented Oct 16, 2019

The current version of the library fails to run due to an invalid import when the mjs version is used. The bundled es version copies the

import { join } from 'path';

import verbatim. However, node interprets mjs files with a different set of rules. For commonjs interoperability, node only allows default imports, as explained in the documentation. Because of that, upon running cypress with cypress-commands, we get:

Oops...we found an error preparing this test file:

  src/support/index.ts

The error was:

/workspace/node_modules/cypress-commands/dist/cypress-commands.mjs 851:29-33
Can't import the named export 'join' from non EcmaScript module (only default export is available)
 @ ./src/support/index.ts
 @ multi ./src/support/index.ts

The solution is to change the named join import to a path default import:

import path from 'path';
  • Tests
  • Documentation
  • Type definitions
  • Ready for merge

@alejo90
Copy link
Contributor Author

alejo90 commented Oct 16, 2019

The CI is failing because it's unable to install a linux dependency. Please take a look.

@Lakitna
Copy link
Owner

Lakitna commented Oct 17, 2019

It seems to have been a weird issue with the CI itself. Rerunning solved it.

Is this the only issue you came across when trying to use the .mjs file? I haven't used it myself, but if there are any issues with I'd like to tackle them all at once.

@alejo90
Copy link
Contributor Author

alejo90 commented Oct 17, 2019

Yes, that's the only issue with mjs. After making that change, I was able to run my tests.

@Lakitna
Copy link
Owner

Lakitna commented Oct 17, 2019

Awesome, I'll publish 0.3.1 right away so you can get going :)

@Lakitna Lakitna merged commit 1219140 into Lakitna:develop Oct 17, 2019
@Lakitna Lakitna mentioned this pull request Oct 17, 2019
@alejo90 alejo90 deleted the mjs-import-error branch October 17, 2019 21:28
@alejo90
Copy link
Contributor Author

alejo90 commented Oct 17, 2019

Thank you very much!

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

Successfully merging this pull request may close these issues.

2 participants