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

not all expected modern JavaScript syntax supported #1183

Open
thescientist13 opened this issue Nov 10, 2023 · 2 comments · May be fixed by #1321
Open

not all expected modern JavaScript syntax supported #1183

thescientist13 opened this issue Nov 10, 2023 · 2 comments · May be fixed by #1321
Labels
bug Something isn't working CLI v0.31.0
Milestone

Comments

@thescientist13
Copy link
Member

thescientist13 commented Nov 10, 2023

Summary

Noticed that some basic JavaScript syntax features seems to be breaking though they should already be supported, like private class fields and static blocks.

// static blocks
class SlideViewer extends HTMLElement {
  static observedAttributes = ['slide'];

   //. ...
}

customElements.define('slide-viewer', SlideViewer);
// private 
class Counter extends HTMLElement {
  #x = 0;

  clicked() {
    this.#x++;
    window.requestAnimationFrame(this.render.bind(this));
  }

  constructor() {
    super();
    this.onclick = this.clicked.bind(this);
  }

  connectedCallback() { this.render(); }

  static observedAttributes = ['slide'];

  render() {
    this.textContent = this.#x.toString();
  }
}
window.customElements.define('num-counter', Counter);

The result of running the build will throw an error from Rollup

bundling static assets...
TypeError: baseVisitor[type] is not a function
    at c (file:///Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/acorn-walk/dist/walk.mjs:23:22)
    at base.ClassBody (file:///Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/acorn-walk/dist/walk.mjs:435:5)
    at c (file:///Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/acorn-walk/dist/walk.mjs:23:22)
    at base.Class (file:///Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/acorn-walk/dist/walk.mjs:428:3)
    at c (file:///Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/acorn-walk/dist/walk.mjs:23:22)
    at base.ClassDeclaration.base.ClassExpression (file:///Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/acorn-walk/dist/walk.mjs:424:80)
    at c (file:///Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/acorn-walk/dist/walk.mjs:23:22)
    at Object.skipThrough (file:///Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/acorn-walk/dist/walk.mjs:180:37)
    at c (file:///Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/acorn-walk/dist/walk.mjs:23:22)
    at base.Program.base.BlockStatement (file:///Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/acorn-walk/dist/walk.mjs:192:5) {
  code: 'PLUGIN_ERROR',
  plugin: 'greenwood-import-meta-url',
  hook: 'transform',
  id: '/Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/greenwood-starter-presentation/dist/components/presenter-mode.js',
  watchFiles: [
    '/Users/owenbuckley/Workspace/github/greenwood-starter-presentation/.greenwood/758806547.js',
    '/Users/owenbuckley/Workspace/github/greenwood-starter-presentation/.greenwood/1377133907.js',
    '/Users/owenbuckley/Workspace/github/greenwood-starter-presentation/.greenwood/12042177.js',
    '/Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/greenwood-starter-presentation/dist/components/presenter-mode.js',
    '/Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/greenwood-starter-presentation/dist/components/slide-list.js',
    '/Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/greenwood-starter-presentation/dist/components/link-target.js',
    '/Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/greenwood-starter-presentation/dist/components/iframe-key-capture.js',
    '/Users/owenbuckley/Workspace/github/greenwood-starter-presentation/node_modules/greenwood-starter-presentation/dist/components/slide-viewer.js'
  ]
}
error Command failed with exit code 1.

Here's a little repo with some of these features demonstrated - https://github.com/thescientist13/greenwood-ihtml/tree/latest-acorn-features

Details

At the time of this writing these should all be Stage 4 in ECMA and supported in all browsers, so not sure what gives? 🤷‍♂️


I notice we are inconsistently configuring Acorn with different flags, one still stuck in 2020!

We should probably make this a lib that can export that configuration everywhere. Additionally, we should make sure that in places we are walking a JS AST, there's no chance we could be processing TS and thus breaking Acorn.

@thescientist13
Copy link
Member Author

As an aside, I wonder if this actually is something Greenwood should support, in exposing it's internals like this, instead of #550 ? I guess its a question of the lag between standards, and that standard showing up in the parsing tools, like Acorn (JS) and CSSTree (CSS). 🤔

@thescientist13 thescientist13 moved this from 🔖 Ready to ➡ DEFERRED in [Greenwood] Phase 9 - Standards and Conventions Aug 10, 2024
@thescientist13 thescientist13 changed the title static property initializations break during bundling not all expected JavaScript syntax supported during build time optimizations Oct 10, 2024
@thescientist13
Copy link
Member Author

thescientist13 commented Oct 10, 2024

Doing some research yielded these related results

Which seem to indicate that as of >= v2.79.0, Rollup should be supporting, so not sure what gives? I made sure to add a resolution to my project and confirm the latest 2.x version of Rollup is installed in this project and also tried manually setting the ECMA version in rollup.config.js

const ast = this.parse(await response.text(), { ecmaVersion: 'latest' });
➜  greenwood-starter-presentation git:(enhancement/issue-25-remove-lit-dependency) ✗ yarn why rollup
yarn why v1.22.19
[1/4] 🤔  Why do we have the module "rollup"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "[email protected]"
info Reasons this module exists
   - "@greenwood#cli" depends on it
   - Hoisted from "@greenwood#cli#rollup"
info Disk size without dependencies: "6.46MB"
info Disk size with unique dependencies: "6.46MB"
info Disk size with transitive dependencies: "6.46MB"
info Number of shared dependencies: 0
✨  Done in 0.11s.

I'm not sure if #1087 will help with this, though I think they switched to SWC in v3? Would be good to test this out there.

Putting this code into the current version of their REPL seems to work at least 👀
Screenshot 2023-11-09 at 8 44 46 PM

As a work around if someone really needed to, they could probably use our Babel plugin (or create their own using esbuild, SWC, etc) that should work as a stop gap.

Or if doing something like Web Components, just the getter version

static get observedAttributes() {
  return ['slide'];
}

I think this is actually an acorn issue, as I see there is this repo for what appears to be the features we're looking for
https://github.com/acornjs/acorn-stage3

So not sure if we just need to add that for now?

@thescientist13 thescientist13 changed the title not all expected JavaScript syntax supported during build time optimizations not all expected modern JavaScript syntax supported Oct 10, 2024
@thescientist13 thescientist13 moved this from 🔖 Ready to 🏗 In progress in [Greenwood] Phase 10 - Ecosystem Compat Nov 14, 2024
@thescientist13 thescientist13 linked a pull request Nov 14, 2024 that will close this issue
5 tasks
@thescientist13 thescientist13 linked a pull request Nov 14, 2024 that will close this issue
5 tasks
@thescientist13 thescientist13 moved this from 🏗 In progress to 👀 In review in [Greenwood] Phase 10 - Ecosystem Compat Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CLI v0.31.0
Projects
Status: 👀 In review
Development

Successfully merging a pull request may close this issue.

1 participant