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

fix: esm: runtime issue with importing EventEmitter: using named import #10

Closed
wants to merge 4 commits into from

Conversation

btakita
Copy link

@btakita btakita commented Jul 19, 2021

fixes #9

dist/CheapWatch.cjs & dist/CheapWatch.mjs are bundled in this PR. Feel free to convert back to original file names if preferred.

I'm also using the rollup-plugin-typescript2 plugin to fix a build issue inside of a pnpm multirepo with cheap-watch installed as a git submodule.

The dist/CheapWatch.mjs top line is now import { EventEmitter } from 'events'; instead of import * as EventEmitter from 'events';

@btakita
Copy link
Author

btakita commented Jul 20, 2021

Fixes #9

rollup.config.js Outdated Show resolved Hide resolved
@benmccann
Copy link

Putting "Fixes #9" in a comment won't link the PR to the issue. You need to do it in the description. Can you edit the description so that "Fixes #9" isn't in a code block or quoted or anything like that, but is just plain text

@btakita
Copy link
Author

btakita commented Jul 22, 2021

Got it. I was adding the "fixes #9" with an indent to the commit which caused it to be quoted. Will update my process.

@btakita
Copy link
Author

btakita commented Jul 22, 2021

@benmccann Thank you for accepting this merge. I want to point out that "type": "module" was added to package.json. Is cheap-watch ready to move over to ESM by default? There is cjs & mjs handling added but I was not able to test this from a cjs module.

@btakita
Copy link
Author

btakita commented Jul 22, 2021

I updated test.js to ESM.

@benmccann
Copy link

I think that the important part of this has now been merged and released: #4

@Conduitry
Copy link
Owner

Besides the change in #4, I don't think the other changes here are something I want to do. I don't want "type": "module" because I don't want to exclude earlier versions of Node.

If you want to install this as a Git submodule rather than using npm, I'd recommend installing the tags directly (rather than master), as they point to already-built copies of the code - e.g. https://github.com/Conduitry/cheap-watch/tree/v1.0.4

@Conduitry Conduitry closed this Sep 7, 2021
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.

ESM: TypeError: Class extends value [object Module] is not a constructor or null error
3 participants