swc-plugin icon indicating copy to clipboard operation
swc-plugin copied to clipboard

Duplicate import aliases bug

Open T9-Forever opened this issue 2 years ago • 5 comments

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 avatar Apr 07 '24 09:04 T9-Forever

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)
    )
}

T9-Forever avatar Apr 07 '24 10:04 T9-Forever

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'

timofei-iatsenko avatar Apr 08 '24 07:04 timofei-iatsenko

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.

T9-Forever avatar Apr 09 '24 10:04 T9-Forever