ghostery / adblocker

Efficient embeddable adblocker library
https://www.ghostery.com
Mozilla Public License 2.0
750 stars 93 forks source link

CommonJS module support #4054

Open IanKrieger opened 4 days ago

IanKrieger commented 4 days ago

I've encountered an issue with the release v1.27.10 and wanted to seek clarification. In this version, the module structure for all monorepo packages was changed from CommonJS to ECMAScript (PR: https://github.com/ghostery/adblocker/pull/3924/).

The new version will no longer compile, with an error stating:

TS1479: The current file is a CommonJS module whose imports will produce require calls; however, the referenced file is an ECMAScript module and cannot be imported with require. Consider writing a dynamic import(@cliqz/adblocker)' call instead.
To convert this file to an ECMAScript module, change its file extension to .mts, or add the field `type: module` to
~/Projects/my-project/package.json

I am currently using the library to validate URL's server side against the block list, after the URL has been submitted via a form. I am using TypeScript & Node, and can not currently switch to use ESM.

If the answer is "we are no longer supporting CommonJS," that is understandable. We are prepared to stay on v1.27.3 for the time being and can upgrade after our project's switch to ESM.

seia-soto commented 2 days ago

Hello @IanKrieger ,

It'd be great if we can get your project setup to reproduce the problem correctly. What we'll need are the followings:

  1. target, module, and moduleResolution field in tsconfig.json
  2. The command used to compile the typescript

I was able to compile the library using the following setup:

{
  "compilerOptions": {
    "target": "es2016",
    "module": "commonjs",
    "moduleResolution": "node10",
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "strict": true,
    "skipLibCheck": true,
  }
}
PS > pnpm tsc; node .\index.js 
{
  exception: undefined,
  filter: NetworkFilter {
    csp: undefined,
    filter: undefined,
    hostname: 'domain.tld',
    mask: 335609855,
    domains: undefined,
    denyallow: undefined,
    redirect: undefined,
    rawLine: undefined,
    id: 3957564394,
    regex: undefined
  },
  match: true,
  redirect: undefined,
  metadata: undefined
}

If you're using tapjs/tsimp, please note that some fields are hardcoded at all: see https://github.com/tapjs/tsimp?tab=readme-ov-file#module-moduleresolution-and-other-must-haves

IanKrieger commented 1 day ago

Thanks for the reply @seia-soto

Here is my tsconfig:

  "compilerOptions": {
    "lib": ["es2023"],
    "module": "Node16",
    "target": "es2022",
    "strict": false,
    "esModuleInterop": true,
    "skipLibCheck": true,
    "forceConsistentCasingInFileNames": true,
    "removeComments": true,
    "sourceMap": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "outDir": "dist",
    "resolveJsonModule": true,
    "incremental": false,
    "importHelpers": true,
    "isolatedModules": true,
    "strictBindCallApply": true,
    "strictFunctionTypes": true,
    "noImplicitThis": true,
    "noImplicitOverride": true,
    "noFallthroughCasesInSwitch": true,
    "allowUnreachableCode": false,
    "allowUnusedLabels": false,
    "useUnknownInCatchVariables": true,
    "strictPropertyInitialization": false,
    "noImplicitAny": true
  },

I just use tsc to compile.

seia-soto commented 1 day ago

Hi @IanKrieger ,

Thank you for providing the information. I was able to reproduce the issue but I think this is an expected behavior by the typescript compiler. But to be sure, this is fixable in the library level.

From the tsconfig.json you provided, I could see the module value is set to node16 which is an equivalent to nodenext. According to TypeScript Handbook, your package is compiled to CommonJS (which is your intent) in this module setup because:

However, there's "type": "module" set and the file extension of type export is set to d.ts (not d.cts) in package.json when typescript compiler sees our library. Therefore, our library is served as ESM by your module setup and the interoperability rule throws the error as you're referencing ES module (adblocker library) from CJS module (your package). You can see "When a CommonJS module references an ES module" section under "Interoperability rules" in the handbook link above. You can confirm this by removing "type": "module" from our library under node_modules directory.

In conclusion, I suggest you to change the module value in your tsconfig.json to commonjs at the moment to force typescript compiler to resolve all modules in commonjs manner (but this is not recommended by typscript team). Otherwise, you can patch the package by using various toolings around your package manager. Doing one of following will make typescript compiler to resolve our library as commonjs:

I will also make a separate PR to solve this issue in the library level, so you don't need any other actions in the future. Thank you for reporting and the contribution, all of your efforts.

I hope you have a great day.

Best