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

Latest version 3.2.0 contains breaking changes #177

Closed
Evertvdw opened this issue Jul 10, 2023 · 10 comments
Closed

Latest version 3.2.0 contains breaking changes #177

Evertvdw opened this issue Jul 10, 2023 · 10 comments

Comments

@Evertvdw
Copy link

We were using this packages also in a Node context (v16) which did a require of this packages. Using the latest version this gives the following error Error [ERR_REQUIRE_ESM]: require() of ES Module /usr/share/www/node_modules/@extractus/oembed-extractor/src/main.js breaking our application pretty catostrophically.

I don't think this should have been a minor release, it's find to move to ESM modules only but not when doing this as a minor release. I have now pinned the version, but I guess others will have similar issues.

@ndaidong
Copy link
Collaborator

@Evertvdw sorry for the inconvenience, this lib has been tested in Node.js 16. Could you provide more info about your environment and the way you load it into your script?

@Evertvdw
Copy link
Author

I'm using Quasar which uses Vite under the hood and the error occurs on SSR when the server-entry.js file tries to require() this package. The server-entry is compiled as commonJS I think.

@ndaidong
Copy link
Collaborator

@Evertvdw it seems that your (generated) source still use CommonJS? Does it have config to change to ES6 format? I think that every platform now supports ES6 Modules import as well.

@Evertvdw
Copy link
Author

It does not yet, It is part of a framework that I don't have control over. They are working on compiling to ESM but that is not yet available. It is fine for me to use a fixed version btw I just created this ticket to signal that this problem exists, so that it might help others. And maybe it is a good idea to release a new major version for this change instead of doing it as a minor release.

@ndaidong
Copy link
Collaborator

@Evertvdw thank you. Is it good to add another minor release with CJS version generated for this case?

@Evertvdw
Copy link
Author

Yes I think so, that way people will not experience this breaking change if they happen to use this package inside commonjs. Thanks for the quick response!

@ndaidong
Copy link
Collaborator

ok, I will do that.

ndaidong added a commit that referenced this issue Jul 10, 2023
- Add CommonJS version (issue #177)
@ndaidong ndaidong mentioned this issue Jul 10, 2023
@ndaidong
Copy link
Collaborator

@Evertvdw done, please test v3.2.1

@actraiser
Copy link

actraiser commented Jul 11, 2023

when using 3.2.1, it will not locate the types properly with either import method:

import { extract, setProviderList } from '@extractus/oembed-extractor';

as well as...

const oembed = await import('@extractus/oembed-extractor');

will produce the error:

Could not find a declaration file for module '@extractus/oembed-extractor'. '/Users/......../node_modules/.pnpm/@extractus[email protected]/node_modules/@extractus/oembed-extractor/src/main.js' implicitly has an 'any' type.
api:dev:local-ssl: There are types at '/Users/......./node_modules/@extractus/oembed-extractor/index.d.ts', but this result could not be resolved when respecting package.json "exports". The '@extractus/oembed-extractor' library may need to update its package.json or typings.

Pinning to version 3.2.0 will make the second (dynamic) import work again. I use the package in a nestjs application within a turborepo if that matters.

Greets
-act

@ndaidong
Copy link
Collaborator

@actraiser thanks, so we will go with v4.x.x, as 3.2.1 is the last version that supports CommonJS!

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

No branches or pull requests

3 participants