search

LDflex / Query-Solid

Simple access to data in Solid pods through LDflex expressions
https://solid.github.io/query-ldflex/
MIT License
66 stars 15 forks source link

Using version 2.7.0 breaks my app's test suite #47

Closed james-martin-jd closed 4 years ago

james-martin-jd commented 4 years ago

On my app, I was using 2.6.0 but recently tried upgrading to 2.7.0. This caused all test suites for pages using ldflex to fail with the following error message:

  appPath/node_modules/@solid/query-ldflex/node_modules/ldflex/lib/MutationFunctionHandler.js:78
            for await (const item of arg) objects.push(this.extractObject(pathData, path, item));
                ^^^^^

    SyntaxError: Unexpected reserved word
RubenVerborgh commented 4 years ago

So what happened between 2.6.0 and 2.7.0 is https://github.com/solid/query-ldflex/commit/42c57e316ce711b88ad6ca21ff17bcedae33ce04: the npm module now exposes the ES6 source code in addition to the transpiled ES5 code. This is done such that consumers that understand ES6 can use the more optimal non-transpiled versions. So it's not a bug, it's a feature—but one that the app needs to deal with appropriately.

Also, we are now building for Node 10 and up, and for recent browsers: https://github.com/solid/query-ldflex/commit/42c57e316ce711b88ad6ca21ff17bcedae33ce04#diff-7f5f3113150dddb0b76c2a8714a14e00

To help you further, I'd need to know what Node version you are using, and what configuration to build the app? (A link to the repo can also do.)

james-martin-jd commented 4 years ago

Node version is v8.9.0 and the repository is https://github.com/inrupt/generator-solid-react/tree/develop/generators/app/templates that's using it. I believe we're loading it up with webpack, so maybe this is just a simple webpack config change. I've rolled back to 2.6.0 for the time being.

RubenVerborgh commented 4 years ago

Node version is v8.9.0

That's it right there; is there a reason to stick to a relatively old Node version? 12 is the LTS now, 8 is going out of maintenance the end of the year: https://nodejs.org/en/about/releases/

james-martin-jd commented 4 years ago

Well - yes, and I actually also have v12.10.0, we have a legacy app that requires 8.9.0 and I just forgot to switch back. This is the first issue I've had with 8.9.0 in.. must be about 6 months?

RubenVerborgh commented 4 years ago

Yeah, Node versions go fast. We can make it work on 8.x if we have to—but I don't think there is an inherent necessity. Let me know if that changes.

RubenVerborgh commented 4 years ago

I'll close this for now; let me know if anything changes.