Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lookup_char_* can fail on certain type of Typescript node types (enum, etcs) #5535

Open
kwonoj opened this issue Aug 17, 2022 · 10 comments
Open

Comments

@kwonoj
Copy link
Member

kwonoj commented Aug 17, 2022

Current bahavior

To allow optimizing binary size, SWC invokes custom transforms (for custom binary) and plugins after applying resolver, decorators, typescript::strip.

This had great impact on reducing binary size, but because of this plugin or custom transforms may accept BytePos(0), which is reserved for compiler-generated nodes, for spans of those AST nodes.

Currently, changing it to pass AST to plugin before applying decorators is considered. But if we do so the plugins which use noop_visit_mut_type!() for size optimization will be broken.

@kwonoj
Copy link
Member Author

kwonoj commented Aug 17, 2022

@kdy1 Thoughts on how to resolve this? it blocks coverage instrumentation for some typescript files.

@kdy1 kdy1 added this to the Planned milestone Aug 17, 2022
@kdy1
Copy link
Member

kdy1 commented Aug 17, 2022

Maybe we can pass the whole AST with typescript types to the plugin but then plugins using noop_visit_mut for optimization will be broken.

@kwonoj
Copy link
Member Author

kwonoj commented Aug 17, 2022

Maybe plugin would silently ignore these, in most cases lookup source would be needed for the real codes only. In that case the issue is lookup_char traps if it fails to lookup - which plugin does not have way to gracefully handle this at all.

@kdy1 kdy1 changed the title Plugin cannot lookup pos for the enums lookup_char_* can fail on certain type of Typescript node types (enum, etcs) Aug 17, 2022
@kdy1 kdy1 modified the milestones: Planned, Next major Aug 17, 2022
@sam-goodwin
Copy link

this plugin or custom transforms may accept BytePos(0)

In what cases will this happen? Can plugins rely on the spans correctly mapping back to the source?

@kwonoj
Copy link
Member Author

kwonoj commented Sep 8, 2022

In what cases will this happen?

If plugin receives already transformed AST from prior transform / or core transforms which have dummy spans.

@sam-goodwin
Copy link

sam-goodwin commented Sep 8, 2022

If plugin receives already transformed AST from prior transform

Not precisely sure what you mean by this. Below is my configuration - I only have my plugin configured, but I'm transforming typescript to JS using the built-in SWC transform. Will the spans my plugin sees map correctly back to the source that created them? It's not clear to me if the ts->js transform runs before or after my plugin - I assume before.

{
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "dynamicImport": false,
      "decorators": false,
      "hidden": {
        "jest": true
      }
    },
    "transform": null,
    "target": "es2021",
    "loose": false,
    "externalHelpers": false,
    "experimental": {
      "plugins": [
        [
          "<my-plugin>",
          {}
        ]
      ]
    }
  },
  "minify": true,
  "sourceMaps": "inline",
  "module": {
    "type": "commonjs"
  }
}

@kwonoj
Copy link
Member Author

kwonoj commented Sep 8, 2022

That is the case issue's main body describes as core transform strips out ts codes.

@sam-goodwin
Copy link

Thanks for the quick responses!

Ok, so just to be clear. A plugin can rely on mapping a Span to the LineCol when the Span is not at BytePos(0). In this case, the LineCol will point to the code within the original TypeScript code. This would be the expected behavior and I can't imagine it being useful any other way.

@kwonoj
Copy link
Member Author

kwonoj commented Sep 8, 2022

A plugin can rely on mapping a Span to the LineCol when the Span is not at BytePos(0)

Yes, that's my understanding as well.

This would be the expected behavior and I can't imagine it being useful any other way.

I'd say yes for the expected behavior, not sure being useful any other way exactly means.

@sam-goodwin
Copy link

Thank you - that clears things up for me.

not sure being useful any other way exactly means

Oh - I just meant that it wouldn't make sense if a non-zero Span didn't point back to source that created that node.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants