Bind function values to node so that they have the correct this when called

[G-Rath]

This allows methods on nodes to be called; specifically, the majority of methods on TypeScript Nodes can now be called.

There is a better way to implement this somewhere, but this was the simplest low-key way I could hack together given that my IDE just can't handle this repo 😂

This includes getSourceFile, which now runs perfectly instead of locking the whole tab up.

[fkling]

Oh my.... this used to work, but I completely broke it when I switched to functional components. Instead of going your way, may I suggest to fix the existing code instead 😬

This line should pass in the current node, not the child node:

astexplorer/website/src/components/visualization/tree/Element.js

Line 204 in afb16cd

parent={value}

value has a different value inside the function than outside. But you can see that the value and parent props should not have the same value.

Here we try to call the function with the correct context:

astexplorer/website/src/components/visualization/tree/Element.js

Line 383 in afb16cd

const computedValue = value.call(parent);

but parent isn't actually pulled from props. I assume this doesn't fail only because window.parent exists 🤦‍♂️

[G-Rath]

Here we try to call the function with the correct context:

I figured that was what was being attempted, but couldn't quite get it working - originally I added parent as an extra parameter to renderChild, but iirc that got troublesome for some reason or another.

As such I could have missed an easy way to get access to the current node; I think if you could confirm where I can get that reference (or the implementation you'd like me to use to get it to that prop i.e parameter, higher-scoped variable, etc) that'll be a great time saver, as that was what I was missing the first time I attempted a fix.

The current node could already be available in really obvious way, but a big part of the problem was that I elected to do the work in my terminal rather than IDE, as the sheer bulk of parsers causes WebStorm to be unusable most of the time as it tries to index all the files 😬

I assume this doesn't fail only because window.parent exists

That is correct - instead everything just fails w/ "this doesn't exist on windows.parent".

[fkling]

Maybe change renderChild to something like

function renderChild(key, value, parent, ...) { ...

and call it with

renderChild(key, value, node, ...)

(keep the const node = value; part you added)
Inside the function element component you just need to add parent to the destructured props:

const {name, value, parent, computed, treeAdapter} = props;

I hope this helps, if not, let me know!

---

as the sheer bulk of parsers causes WebStorm to be unusable most of the time as it tries to index all the files grimacing

😞

The current state is a bit of a mess. The more parsers are added the longer it takes to build the app. I am slowly decoupling parsers from the website code itself... one day things will be better 😉

[G-Rath]

Perfect - that was the implementation I originally went for, but couldn't get working for some reason :/

Also turns out if you call the constructor method, it'll prevent all the other methods from working 😂

---

The current state is a bit of a mess. The more parsers are added the longer it takes to build the app. I am slowly decoupling parsers from the website code itself... one day things will be better 😉

Yeah - while completely understandable it's bit of a pain to the point that @typescript-eslint are considering rolling their own custom build of astexplorer.

Personally, I view astexplorer as sort of the npm version of GodBolt, and think it would be a shame if things fragmented into their own little mini-astexplorers. If there's anything I can do to help, let me know :)

---

Just saw #197 which looks pretty much along the lines of what I was thinking 😂

(I'm too lazy to apply strikeout to everything)
I did have an idea on a way to "easily" separate the parsers out & greatly reduce maintenance overhead for supporting new versions:

My understanding is that the core of astexplorer is reasonably generic and that parsers are supported via a thin transformer (which is not so thin in some cases); so effectively to support a parser you need two things:

• a browser-compatible version of the parser, and
• an "ast explorer wrapper" for the parser (this includes "the version of the parser")

This is something you could source via separate packages, via something like unpkg, which would mean you could just query npm for a list of versions of a package, and that way users could just pick whatever version they wanted.

The actual separate package could be in a couple of ways, such as shipping a field in the package.json of each parser, or an astexplorer.json, since unpkg makes the whole package available over the web and so you could do something like:

if `unpkg.com/<package>@<version>/astexplorer.json` exists:
  /* ...download, parse, apply... */
else:
  /* check for @astexplorer/<package>-mapper@<version> */
fi

astexplorer.json could have information like the path to the browser version of the package, the name of the actual "mapper package" so that the mapper doesn't need to be shipped in the actual parser, and more?

There's a number of different ways of doing the actual mapper-package part, but in particular the big win is that even if you did something like @astexplorer/<parser>-mapper, you could invite core maintainers of parsers to the repo for their parser and thus let them manage that as they wish.

While it would take a bit of planning, I don't really see a big problem with the above - aside from it could be horrible at scale?

If it sounds feasible, I'm happy to take a whack at it, just by first seeing about hacking in using a package from unpkg for version selection :)

[fkling]

Yeah - while completely understandable it's bit of a pain to the point that @typescript-eslint are considering rolling their own custom build of astexplorer.

Personally, I view astexplorer as sort of the npm version of GodBolt, and think it would be a shame if things fragmented into their own little mini-astexplorers. If there's anything I can do to help, let me know :)

Thanks for pointing that out! And yes, fragmentation is exactly what I want to avoid. I'd rather have people contribute back and improve astexplorer to make it better for everyone.

And I can also understand that the current process is frustrating. But I'm also just one person and I find it a bit unfair to complain and say "lets do our own thing" instead of approaching me and offering help/suggestions.

Also I'm not sure they even can do that with the code since I haven't added a license to it (exactly for the reason to prevent this, but that is probably a separate discussion).

---

As for your suggestion: There is a lot to unpack there, but first let me tell you: thank you! I really appreciate your words of confidence and your help!

You are spot on about the "structure" of the parsers and I like your idea. While I have considered how to separate the parsers from astexplorer, how to handle the "wrappers" wasn't that clear.

Here is the problem how I see it:

Each parser comes in different versions (it could that only the latest one is really relevant, but beta versions is something that might be useful to support, so we should assume multiple versions). Each version could have a different API (unlikely) and different configuration options (more likelt).

The wrapper (facade?) is the mediator between the parser and astexplorer. It takes input and options, passes it to the parser, converts the parser result to something astexplorer understands and returns to astexplorer.
And this is the part where I'm struggling with: Given different parser versions, there must either be a wrapper per version or the wrapper must be able to handle multiple versions.
At the same time the interface between the wrapper and astexplorer can change. So the wrapper will have to be kept up to date in order to work with astexplorer. I don't expect the interface to change often, but it something that needs to be kept in mind.

So this all sounds like a maintenance nightmare. But maybe it isn't because that maintenance burden is spread across more people, so maybe not really an issue.

But imagine I want to change something about astexplorer, e.g. how we traverse the tree, which would require the wrappers to be updated. How would such a change be coordinated? I can't just make the change and wait for others to update the wrappers. No one would be able to use astexplorer until the wrappers were updated. And if there are multiple wrappers for different versions of the parsers, all of them would need to be updated too.

Again, maybe it's not as much of an issue, I know that I'm over--complicating things sometimes.

---

About sourcing parsers from npm: My original concern was security. If we would source any package that has a specific keyword or something, we could be pulling in and running arbitrary code in people's browsers.

But that could be mitigated by having a whitelist of npm packages. And building the registry from that could work as you suggested (maintainers including a special file or package.json field).
I'm not quite sure yet how we would specify which versions of a parser should be available though...

Additionally, since maintaining separate packages for wrappers and stuff might not be a good option for everyone, there would be second "registry" for parsers that are "bundled" with astexplorer, basically what we have now. This could a be good option for smaller parsers.

My plan was to also provide the option to add custom registries as a client side setting. E.g. babel could have their own registry with all the experimental builds. People would have to add it manually to astexplorer (but only once), but I think that's ok.

---

I hope I didn't overwhelm you with my brain dump 😆

[fkling]

Having big parsers provide their own browser build would also avoid situations like #467 (comment) because I don’t know how to configure webpack correctly 😉

I actually have some more concrete plans about the changes I want to being to astexplorer (I already started working on them) and I will share them later when I have a bit more time.

Maybe we should open a new issue to discuss all this, since this PR is about to get closed...

[fkling]

Alright, the changes should be live now!

[G-Rath]

Alright, the changes should be live now!

Woot woot!

And I can also understand that the current process is frustrating. But I'm also just one person and I find it a bit unfair to complain and say "let's do our own thing" instead of approaching me and offering help/suggestions.

I wouldn't take it personally - I think that issue was born of a mix of things: the fact that astexplorer is pretty critical to @typescript-eslint being able to quickly build & grow, vs the share size & maintenance burden you've got since you are just one person, so I view that issue as a result of looking for the path of least resistance.

I would expect if they went ahead w/ building a custom build, they'd abandon it for the v2 or ASTExplorer we've been talking about here as soon as it was out. I also think what we're talking about now is exactly sort of thing that will give communities like @typescript-eslint an entry point to jump into helping.

It also strikes me as the "nice idea - lets put it in an issue so we can claim it's being track and written down somewhere" kind of issue 😂

And this is the part where I'm struggling with: Given different parser versions, there must either be a wrapper per version or the wrapper must be able to handle multiple versions.

At the same time the interface between the wrapper and astexplorer can change. So the wrapper will have to be kept up to date in order to work with astexplorer. I don't expect the interface to change often, but it something that needs to be kept in mind.

I've bolded the two points in what you said that I think highlight a possible way forward.

My thinking is this:

By publishing the wrappers as their own package, the point-of-maintenance shifts away from both the parsers & astexplorer itself - and so those wrappers could be as mess as they like, which I imagine they might be.

But the main thing is once you've "in" the wrapper package you've got free reign to do whatever you want however you want. I think it would be best to just say the versions of the wrapper packages are not locked to astexplorer or the parser - instead astexplorer takes the latest, and that the wrappers handle version resolving themselves:

interface Versions {
  explorer: string;
  parser: string;
}

type AppliesToRanges = Versions;

const pathToBrowserFile = (version: string) => {
  switch (true) {
    case semver.satisfies(version, '1.x'):
      return 'dist/umd/bundle.js';
    case semver.satisfies(version, '2.x'):
      return 'dist/bundle.js';
    default:
      console.warn(`version ${version} is not technically supported!`);
      return 'dist/bundle.js'; // sensible default
  }
};

const loadParser = (version: string) =>
  loadScriptFromUnpkg('@typescript-eslint/parser', pathToBrowserFile(version));

type NativeToExplorerASTTransformer = (
  ast: unknown,
  versions: Versions
) => unknown;

const versionTransformers: Array<{
  appliesTo: AppliesToRanges;
  transform: NativeToExplorerASTTransformer;
}> = [
  {
    appliesTo: {
      parser: '1.x',
      explorer: '*'
    },
    // we returned an array within an array for some reason in this version
    transform: ast => ast[0]
  },
  {
    appliesTo: {
      parser: '*',
      explorer: '<= 1'
    },
    // previous versions of astExplorer required an object w/ the parser name
    transform: ast => ({
      parserName: '@typescript-eslint',
      ast
    })
  }
];

const transformResultsForExploring = (ast: unknown, versions: Versions) =>
  versionTransformers
    .filter(
      // find all the transformers that should be applied to the versions we're using
      transformer =>
        semver.satisfies(versions.explorer, transformer.appliesTo.explorer) ||
        semver.satisfies(versions.parser, transformer.appliesTo.parser)
    ) // apply each transformer in order to the ast
    .reduce((ast, transformer) => transformer.transform(ast, versions), ast);

const parse = async (contents: string, versions: Versions) => {
  const parser = await loadParser(versions.parser);
  const results = await parser.parse(content);
  const ast = await transformResultsForExploring(results, versions);

  return ast;
};

To be honest, my biggest concern is the script loading part, as theres a number of ways it could break resolving around packages having browser compatible - it's not strictly our problem, but that's the area I see as that'll have the most hacky & maintenance depending on if package maintainers play ball.

I think one of the critical parts is that wrappers shouldn't attempt to be mapping across versions - the "latest" of a wrapper should be expected to be able to handle whatever is put to it; otherwise you you'll end up in a massive maintenance nightmare.

I can't just make the change and wait for others to update the wrappers. No one would be able to use astexplorer until the wrappers were updated. And if there are multiple wrappers for different versions of the parsers, all of them would need to be updated too

While that should be somewhat covered by the above, I think a big part of that also just belongs to the maintainer as they're choosing to source a wrapper.

I would say it should be encouraged to have "explorer core maintainers" such as yourself (and me...?) added to the repos of wrappers for this reason too - that way you could pop in and say "hey I'm making this change to astexplorer, here's what needs to happen ".

Eitherway, I think that's a relatively unavoidable cost that can be reduced in a number of different ways, but will always be the case b/c of the nature of ASTExplorer :)

About sourcing parsers from npm: My original concern was security. If we would source any package that has a specific keyword or something, we could be pulling in and running arbitrary code in people's browsers.

I think this isn't a big deal in the similar way it's not for CodeSandbox, or other such apps - the key is to make sure it's clear we take no responsibility for things going wrong if users choose to target packages & versions we've not cleared as "explorer safe".

The explorer should contain a white-ish list of packages that are considered "core" & thus friendly - so what it currently does, but I think versions should not be limited to that list otherwise it defeats the whole point.

I think adding the ability to target arbitrary packages shouldn't be the focus for now, since it's not nearly as big a maintenance burden as version targeting (nor is it that common for there to be new parsers, since they require transforming anyway, etc etc).

I hope I didn't overwhelm you with my brain dump 😆

I love braindumps, so feel free to keep them coming 😄

Maybe we should open a new issue to discuss all this, since this PR is about to get closed...

Yeah, I think that might be best 😂

[G-Rath]

Honestly, with the code I posted above, and the size of ASTExplorer, I'm thinking maybe its worth actually making a new project as a prototype V2, focusing on @typescript-eslint w/ a wrapper implementation?

That way you're not trying to shoehorn a new system and don't have to worry about breaking anything - the explorer-parser-bridge (as I'm now going to call it, as that's such an awesome name) doesn't need to be published, just isolated so that it can be picked out later.

[G-Rath]

So I was able to hack in loading versions of typescript via:

async loadParser(callback, version = '3.7.3') {
    const url = `https://unpkg.com/typescript@${version}/lib/typescript.js`;
    const code = await (await fetch(url)).text();

    const parser = Function(`${code}; return ts;`)();

    const _ts = parser;
    // workarounds issue described at https://github.com/Microsoft/TypeScript/issues/18062
    for (const name of Object.keys(_ts.SyntaxKind).filter(x => isNaN(parseInt(x)))) {
        const value = _ts.SyntaxKind[name];
        if (!syntaxKind[value]) {
            syntaxKind[value] = name;
        }
    }

    window.ts = parser;                                                                                                                                                                                                                                                                                                                                   
    callback(_ts);
  },

This has given me a couple of thoughts:

• Security isn't something I think we need to worry about too much, as I think this is a situation where it's the general nature of what the application is doing, and it's for use by developers - so long as we put appropriate notice where it needs to be, and attempt to gracefully fail for things like fs access, it shouldn't be anymore a problem than us bundling the packages ourselves like what's currently being done.
• We will need to ship a mini-webpack/browserify to load things purely client side, or else publish rolled up versions of each parsers.
  • This could actually be alright, b/c then we'd have a whitelist while still letting the community maintain their parsers - i.e @typescript-eslint is a mono-repo and all setup for package publishing so it would be completely fine for them to just have a parser-astexplorer package that gets published alongside the standard parser package.
• We should be async everywhere, which I think would be done easier by rebuilding rather than refactoring - for example, I can't really hack in version selection, as the actual version number is tied to a sync variable exported by js/typescript.js - if we say that things are async as much as possible, then it'll make it super easy to deal w/ whatever comes around.