extractus / oembed-extractor

Extract oEmbed data from given webpage
https://extractor-demos.pages.dev/oembed-extractor
MIT License
104 stars 44 forks source link

Latest version 3.2.0 contains breaking changes #177

Closed Evertvdw closed 1 year ago

Evertvdw commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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

Evertvdw commented 1 year ago

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 commented 1 year ago

ok, I will do that.

ndaidong commented 1 year ago

@Evertvdw done, please test v3.2.1

actraiser commented 1 year ago

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+oembed-extractor@3.2.1/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 commented 1 year ago

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