adaltas / node-csv

Full featured CSV parser with simple api and tested against large datasets.
https://csv.js.org
MIT License
4.05k stars 267 forks source link

v5 is no longer isomorphic: the same import can't be used in the browser & server #317

Closed bohendo closed 1 year ago

bohendo commented 2 years ago

Describe the bug

I'm writing a library that depends on csv-parser that I'd like to use both server & client side. I successfully integrated csv-parse@4.16.3 but upgrading to csv-parse@5.x.x breaks either the browser or server depending on how I import.

To Reproduce

Importing csv-parse like this: import { parse as csv } from 'csv-parse/browser/esm/index.js'; breaks the server with the following message:

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /root/node_modules/csv-parse/dist/esm/sync.js
require() of ES modules is not supported.
require() of /root/node_modules/csv-parse/dist/esm/sync.js from /root/modules/transactions/dist/bundle.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename sync.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /root/node_modules/csv-parse/package.json.

Importing csv-parse like this: import { parse as csv } from "csv-parse/sync"; breaks the browser with the following message:

_stream_readable.js:46 Uncaught Error: Cannot find module 'buffer'
    at webpackMissingModule (_stream_readable.js:46)
    at Object.../../node_modules/readable-stream/lib/_stream_readable.js (_stream_readable.js:46)
    at Object.options.factory (react refresh:6)
    at __webpack_require__ (bootstrap:24)
    at fn (hot module replacement:61)
    at Object.../../node_modules/stream-browserify/index.js (index.js:28)
    at Object.options.factory (react refresh:6)
    at __webpack_require__ (bootstrap:24)
    at fn (hot module replacement:61)
    at Module.../../node_modules/csv-parse/lib/index.js (ResizeableBuffer.js:65)

I've been unable to resolve the browser-based problems with a manual polyfill but maybe I'm not doing it right.

I'll continue using csv-parse@4.16.3 because it still works without any problems but I will be unable to upgrade to 5.x.x until I have some way to import csv-parse into my lib so that my lib still works in both browser & server contexts.

Additional context

I'm using typescript@4.5.4 & rollup@2.63.0 to package my library

wdavidw commented 2 years ago

On the server:

On the browser, you can use the same directives as above if you manage polyfill (or your build system like webpack does), otherwise

There are a lot of examples inside the demo folder.

Kaarel commented 2 years ago
* if using ESM, use `import { parse as csv } from 'csv-parse';` (or `import { parse as csv } from 'csv-parse/sync';`).

On Node with "csv-parse": "5.0.4" the above gives an eslint error

module ".../node_modules/csv-parse/dist/esm/sync" Unable to resolve path to module 'csv-parse/sync'

and running the code gives an error

Unable to resolve some modules: "csv-parse/sync"

However when using the old v4 path import { parse as csv } from 'csv-parse/lib/sync'; everything seems to work.

wdavidw commented 2 years ago

@Kaarel provide the code because I won't be able to help otherwise.

Kaarel commented 2 years ago

Tried to reproduce with a vanilla node app to no avail. The problem occurs on an existing legacy app. So there might be other things interfering. I am just witnessing the above behaviour. I realise this doesn't help much debugging the issue. I tried deleting the node_modules directory and fresh npm install but still same.

wdavidw commented 2 years ago

At the mimimum, I need details about your environment: ESM or CJS, JS or TypeScript, Node or Web

Kaarel commented 2 years ago

The parent framework is Meteor JS 2.6 (the app is server side only though, no web anywhere) The code is a mix of TS and JS. The code that uses node-csv specifically is in JS.

NODE_VERSION=14.18.3 tsconfig.json

{
    "compilerOptions": {
        "target": "ES2018",
        "module": "ESNext",
        "lib": ["ESNext"],
        "allowJs": true,
        "checkJs": false,
        "noEmit": true,

        "strict": true,
        "noImplicitAny": true,
        "strictNullChecks": true,

        "noUnusedLocals": true,
        "noUnusedParameters": true,
        "noImplicitReturns": false,
        "noFallthroughCasesInSwitch": false,

        "moduleResolution": "Node",
        "types": ["node", "mocha"],
        "esModuleInterop": true,
        "preserveSymlinks": true,

        "resolveJsonModule": true,
        "baseUrl": ".",
        "paths": {
            "/*": ["*"]
        }
    },
    "exclude": [
        "./.meteor/**",
        "./packages/**",
        "./node_modules/**"
    ]
}

hope this helps.

kitsunde commented 2 years ago

This is probably hitting a lot of people because it occurs in the default airbnb eslint rules, especially it gets raised via https://www.npmjs.com/package/eslint-plugin-import, but I'm uncertain why exactly. Here's a reproducible example:

index.js

require('csv-stringify/sync');

package.json

{
  "name": "test-stringify-import",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "csv": "^6.0.5"
  },
  "devDependencies": {
    "eslint": "^8.11.0",
    "eslint-config-airbnb-base": "^15.0.0"
  }
}

eslintrc.js

module.exports = {
  env: {
    browser: false,
    commonjs: true,
    es2021: true,
  },
  extends: ['airbnb-base'],
  parserOptions: {
    ecmaVersion: 'latest',
  },
  rules: {
  },
};

Results in:

npx eslint ./

/index.js
  1:9  error  Unable to resolve path to module 'csv-stringify/sync'  import/no-unresolved

✖ 1 problem (1 error, 0 warnings)

The actual import is working just fine though, so it's the eslint plugin not being able to resolve the import.

wdavidw commented 1 year ago

I reproduce the issue. The code run. Only eslint is having an issue resolving ESM imports when the package uses the exports field. It might be related to https://github.com/import-js/eslint-plugin-import/issues/1810. In any case, nothing we can do about it, so I am closing this issue.