adblocker icon indicating copy to clipboard operation
adblocker copied to clipboard

CommonJS module support

Open IanKrieger opened this issue 1 year ago • 3 comments

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.

IanKrieger avatar Jun 28 '24 15:06 IanKrieger

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

seia-soto avatar Jul 01 '24 08:07 seia-soto

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.

IanKrieger avatar Jul 01 '24 16:07 IanKrieger

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:

  • The file extension is one of ts, tsx, js, jsx, and d.ts.
  • The nearest package.json (this should be your package.json in the package root directory) doesn't have "type": "module".

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:

  • Remove "type": "module" from our library
  • Make a copy of dist/types/adblocker.d.ts file and save as adblocker.d.cts (you'll need to edit our library's package.json like the below example)
"exports": {
  "require": {
    "default": "./dist/cjs/adblocker.cjs",
    "types": "./dist/types/adblocker.d.cts"
  },
  "import": "./dist/esm/adblocker.js",
  "types": "./dist/types/adblocker.d.ts"
}

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

seia-soto avatar Jul 02 '24 06:07 seia-soto

@IanKrieger can you please try the latest release? It should work just fine with any Typescript setup.

chrmod avatar Jul 15 '24 09:07 chrmod

Hey @chrmod, it looks like it is working now. Appreciate you both resolving the issue!

IanKrieger avatar Jul 16 '24 12:07 IanKrieger

And thank you for reporting!

chrmod avatar Jul 16 '24 13:07 chrmod