lingui / swc-plugin

A SWC Plugin For LinguiJS
https://www.npmjs.com/package/@lingui/swc-plugin
MIT License
64 stars 12 forks source link

Duplicate import aliases bug #87

Open T9-Forever opened 7 months ago

T9-Forever commented 7 months ago
import { t as $t, t } from '@lingui/macro'

t`Hello`

$t`Hello`

Only the last t variable will be registered in imports_id_map

Maybe need to add a secondary data structure importsBindingMap?

// export name -> local name
importsIdMap: Map<string, string>
// local name -> export name
importsIdMapInverted: Map<string, string>

// local name -> ast node
importsBindingMap: Map<string, Set<Identifier>>

If swc has babel's scope.getBinding, it can maintain the importsBindingMap during the registration phase.

Please extend swc's capabilities~

T9-Forever commented 7 months ago

I've rewritten the babel plugin before, because of the risk of discrepancies between the official swc and babel translations. (Native babel plugin for extract & swc plugin for runtime) It may be that the swc plugin can also go through the following

  public safeAddImportsBinding(name: string, node: Identifier) {
    const st = this.importsBindingMap.get(name) ?? new Set<Identifier>()
    // safeUpdate
    this.importsBindingMap.set(name, st)
    st.add(node)
  }

  public safeHasImportsBinding(name: string, node: Identifier): boolean {
    const st = this.importsBindingMap.get(name) ?? new Set<Identifier>()
    // safeUpdate
    this.importsBindingMap.set(name, st)
    return st.has(node)
  }

  public registerMacroImport(impPath: NodePath<ImportDeclaration>) {
    impPath.node.specifiers.forEach((spec) => {
      if (t.isImportSpecifier(spec)) {
        const imported = spec.imported
        const local = spec.local
        if (t.isIdentifier(imported)) {
          this.importsIdMap.set(imported.name, local.name)
          this.importsIdMapInverted.set(local.name, imported.name)
          const bindng = impPath.scope.getBinding(local.name)
          bindng?.referencePaths.forEach((path) => {
            this.safeAddImportsBinding(local.name, path.node as Identifier)
          })
        }
      }
    })
  }

  public isLinguiIdent(name: string, ident: Identifier): boolean {
    return (
      this.importsIdMap.get(name) === ident.name &&
      this.safeHasImportsBinding(ident.name, ident)
    )
}
timofei-iatsenko commented 7 months ago

babel's version uses native babel's capabilities already and doesn't have this flaw. SWC version, indeed, should be improved for that case.

PS

What is the real usecase of this snippet?

 import { t as $t, t } from '@lingui/macro'
T9-Forever commented 7 months ago

babel's version uses native babel's capabilities already and doesn't have this flaw. SWC version, indeed, should be improved for that case.

PS

What is the real usecase of this snippet?

 import { t as $t, t } from '@lingui/macro'

It's a long story. At first our code only supported English, then I used jscodeshift to convert plain text to lingui syntax.

const renderString = 'Hello';
const renderEle = <div>Hello</div>;

auto translate by script

import {t as $t, Trans} from '@lingui/macro';
const renderString = $t`Hello`;
const renderEle = <div><Trans>Hello</Trans></div>;

With the previous strategy, $t looks safer (since I don't know the history of the code, t may have conflicts). Of course it might look a bit silly to write the $t yourself, if it's not transformed by scripts.

Perhaps because of the large number of alias t in the project, teammates have gotten used to writing them. (Stockholm Syndrome, perhaps...)

However, if there are developers at different levels in the project, there is a risk of an production incident if swc does not convert multiple aliases.

It's hard to find problems in compile detection unless you add eslint rules manually (which I'm currently planning to do). However, as an underlying framework, it is necessary for swc to support basic js syntax.