lodash / babel-plugin-lodash

Modular Lodash builds without the hassle.
Other
1.95k stars 95 forks source link

migrate to isImportOrExportDeclaration #260

Open cube-dan opened 1 year ago

cube-dan commented 1 year ago

Babel has deprecated isModuleDeclaration(); see here.

NOTE: isModuleDeclaration() is now a shim for isImportOrExportDeclaration() and has no purpose other than to generate a TON of these messages:

Trace: `isModuleDeclaration` has been deprecated, please migrate to `isImportOrExportDeclaration`.
    at isModuleDeclaration ( [...] /node_modules/@babel/types/src/validators/generated/index.ts:5869:11)
    at PluginPass.call ( [...] /node_modules/babel-plugin-lodash/lib/index.js:102:44)
    at call ( [...] /node_modules/@babel/traverse/src/visitors.ts:261:21)
    at NodePath._call ( [...] /node_modules/@babel/traverse/src/path/context.ts:34:20)
    at NodePath.call ( [...] /node_modules/@babel/traverse/src/path/context.ts:19:17)
    at NodePath.visit ( [...] /node_modules/@babel/traverse/src/path/context.ts:92:31)
    at TraversalContext.visitQueue ( [...] /node_modules/@babel/traverse/src/context.ts:144:16)
    at TraversalContext.visitSingle ( [...] /node_modules/@babel/traverse/src/context.ts:108:19)
    at TraversalContext.visit ( [...] /node_modules/@babel/traverse/src/context.ts:176:19)
    at traverseNode ( [...] /node_modules/@babel/traverse/src/traverse-node.ts:34:17)
    at traverse ( [...] /node_modules/@babel/traverse/src/index.ts:71:15)
    at transformFile ( [...] /node_modules/@babel/core/src/transformation/index.ts:119:13)
    at transformFile.next (<anonymous>)
    at run ( [...] /node_modules/@babel/core/src/transformation/index.ts:47:12)
    at run.next (<anonymous>)
    at transform ( [...] /node_modules/@babel/core/src/transform.ts:29:20)
    at transform.next (<anonymous>)
    at step ( [...] /node_modules/gensync/index.js:261:32)
    at  [...] /node_modules/gensync/index.js:273:13
    at async.call.result.err.err ( [...] /node_modules/gensync/index.js:223:11)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

Thanks for considering this PR.

indeyets commented 1 year ago

related to #259

JLHwung commented 1 year ago

https://github.com/lodash/babel-plugin-lodash/blob/7d2342a3b849c3c8ef3b2d904b7e2394205466ff/package.json#L45

Can you also bump @babel/types to ^7.21.0? Otherwise this PR will fail with @babel/types < 7.21.

cube-dan commented 1 year ago

Bumped. Thanks for the heads up.

JLHwung commented 1 year ago

@cube-dan Can you re-run npm i and commit the changed package-lock.json?

cube-dan commented 1 year ago

@JLHwung,

I tried the npm i locally but got a really dirty result — that I'm not willing to pollute this PR with. Here's the npm output:

npm WARN old lockfile
npm WARN old lockfile The package-lock.json file was created with an old version of npm,
npm WARN old lockfile so supplemental metadata must be fetched from the registry.
npm WARN old lockfile
npm WARN old lockfile This is a one-time fix-up, please be patient...
npm WARN old lockfile
npm WARN deprecated fsevents@1.2.4: fsevents 1 will break on node v14+ and could be using insecure binaries. Upgrade to fsevents 2.

> babel-plugin-lodash@3.3.4 prepublish
> npm run build

> babel-plugin-lodash@3.3.4 build
> babel src --out-dir lib || true

`isModuleDeclaration` has been deprecated, please migrate to `isImportOrExportDeclaration`
    at Object.isModuleDeclaration (/Users/db/Sites/devilbox/data/www/github/babel-plugin-lodash/node_modules/@babel/types/lib/validators/generated/index.js:3940:35)
    at gatherNodeParts (/Users/db/Sites/devilbox/data/www/github/babel-plugin-lodash/node_modules/@babel/traverse/lib/scope/index.js:71:11)
Successfully compiled 10 files with Babel.

added 2 packages, changed 1 package, and audited 368 packages in 28s

32 vulnerabilities (2 low, 3 moderate, 22 high, 5 critical)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

I've attached the package-lock.json(.txt) file in case you want to take a look at the mess that npm i made.

package-lock.json.txt

JLHwung commented 1 year ago

@cube-dan In that case let's keep this PR simple and let maintainers decide whether they should upgrade the lock file format.

If we don't want to bump the @babel/types dep, an alternative solution (that works for every Babel 7 version) is:

import { isImportDeclaration, isExportDeclaration } from '@babel/types'

if (isImportDeclaration(node) || isExportDeclaration(node)) { ... }
Pearce-Ropion commented 1 year ago

@cube-dan Do you time to take on these changes? I'd be happy to pick this up if you don't.

cube-dan commented 1 year ago

@Pearce-Ropion which changes are you referring to?

Pearce-Ropion commented 1 year ago

@Pearce-Ropion which changes are you referring to?

https://github.com/lodash/babel-plugin-lodash/pull/260#issuecomment-1446558460

cube-dan commented 1 year ago

@Pearce-Ropion I'm not sure where that change is supposed to be made; feel free to jump in. Would it simplify this PR to back out of the change made to the package.json file?

Pearce-Ropion commented 1 year ago

According to https://github.com/lodash/babel-plugin-lodash/pull/260#issuecomment-1446558460 you can avoid using isImportOrExportDeclaration by using a combination of isImportDeclaration and isExportDeclaration. Looking at the their implementation, the result is exactly the same thing. Furthermore, we can revert the change to the package.json which means this plugin will continue to work with older versions of babel.

To ensure you can run tests without polluting the package-lock.json, you can run npm ci --lockfile-version 1 when installing the packages. Then you can run npm run test.