google / clasp

🔗 Command Line Apps Script Projects
https://developers.google.com/apps-script/guides/clasp
Apache License 2.0
4.59k stars 428 forks source link

clasp push fails due to spurious syntax errors that can't be investigated #907

Open Qythyx opened 2 years ago

Qythyx commented 2 years ago

(Note: Non-breaking issues are likely not to be prioritized. Please consider a PR in addition to your issue)

I have a project that used to work fine. I recently updated to a new computer and installed everything fresh, so it is possible that is somehow related, but my underlying problem is still valid.

Expected Behavior

My code can by transpiled to Javascript successfully, but when I run clasp push it fails. Furthermore, the failure message isn't useful. It should either show enough info that can be used to investigate the error, or there should be a way to preserve the generated .gs files so they can be examined directly.

Actual Behavior

When I run clasp push I get errors like Syntax error: ParseError: Unexpected token ; line: 9 file: Documents/Order.gs. This .gs file is not available for to see, so I can't examine what is wrong with line 9. Furthermore, my original code in Typescript, so the difference between my source and this file that has an error is significant.

Specifications

Nu11u5 commented 2 years ago

This should be partially addressed in #857.

In your case "Order.gs" refers to the file that has been transpiled from TypeScript to JavaScript and pushed to GAS (where it was rejected due to a syntax error). Before uploading the original file would be normally named ".ts" or ".js".

I suspect that due to transpiling the reported syntax error will not match up to your TypeScript source. Unfortunately there is no simple solution to fix this.

Your error message will contain a large Json object. The files that are uploaded to GAS will be included as an array of file objects containing escaped strings in error.config.body. You can extract the strings and check the content at the reported line number to see if you can recognize it in your source.

I will see if I can update the PR to output the corresponding source line from the push request which will make this process easier for TypeScript files.

Qythyx commented 2 years ago

@Nu11u5, thanks for the response.

I did notice the large JSON object, but in my case it was not shown in full and said something about being truncated with some number of characters not included (sorry, I'm not at the PC with the issue to get the exact message right now). I hope the fix you linked to helps, but in addition if there was a way to save that large JSON object to a file, without it being truncated, that would be useful too.

Nu11u5 commented 2 years ago

@Qythyx Do you need to see the whole transpiled file or just the lines around the reported error?

Qythyx commented 2 years ago

I think that just seeing the lines around the reported error are enough.

Nu11u5 commented 2 years ago

@Qythyx if you are able try building Nu11u5:add-GaxiosError-object-suppression and see if the output is useful for you.

Qythyx commented 2 years ago

@Nu11u5 , I tried it but it seemed to give the same error, but just showing the Typescript filename instead of the .gs. In my case the error is: ParseError: Unexpected token ; - "/Users/rainer/Projects/BeerApp/SpreadSheetFunctions/src/Documents/Beer.ts:9"

but there is no error there, the beginning lines of the file are shown below. Line 9 in the Style: line, and there is nothing wrong with that.

import { Document } from "./Document";

export interface IBeerDetails {
    Id: string;
    Name: string;
    Description: string;
    Brewery: string;
    Tags: string;
    Style: string;
    Container: string;
    Size: string;
}
Nu11u5 commented 2 years ago

@Qythyx The most recent commit on my branch added a code snippet output which should show you the code that causes the syntax error as sent to GAS, like this:

clasp-syntax-error-code-snippet

Qythyx commented 2 years ago

@Nu11u5, is there an option needed to show the snippet? When I run this is all I get:

❯ clasp push
Push failed. Errors:
ParseError: Unexpected token ; - "/Users/rainer/Projects/BeerApp/SpreadSheetFunctions/src/Documents/Beer.ts:9"

In this case, looking at the code I don't see any errors, and locally transpiling to Javascript is fine. So I wonder if it isn't a real syntax error, which is maybe why the snippet isn't being shown. But that leaves me back at my original problem of not being able to push.

Nu11u5 commented 2 years ago

@Qythyx are you willing to share your code so I can reproduce the issue?

Your error output will only show for a SyntaxError type, but something must not be parsing the code payload correctly.

Qythyx commented 2 years ago

@Nu11u5, sure. What is the best way to share it with you privately?

Nu11u5 commented 2 years ago

Contacted - thanks!

Nu11u5 commented 2 years ago

@Qythyx try the latest clasp code now (the snippet function now looks up the file payload correctly when the path uses a subdirectory).

It looks like your problem is your class field declarations. While GAS supports ES6 classes, it doesn't like field declarations for some reason, and the transpiler doesn't work around this I guess. You can still create empty fields with the constructor via this.field = undefined and define the class and fields types with JSDoc.

Push failed. Errors:
ParseError: Unexpected token ; - "/home/debianuser/Projects/SpreadSheetFunctions/src/Documents/Beer.ts:9"
  Object.defineProperty(exports, "__esModule", { value: true });
  exports.Beer = void 0;
  //import { Document } from "./Document";
  class Beer {
⇒     id;
      PartitionKey = "Beer";
      ABV;
      IBU;
      ImageUrl;
Qythyx commented 2 years ago

@Nu11u5, thanks, that error snippet is definitely better than before.

I understand your explanation about the field declarations being the problem, but I'm very confused of why this is suddenly happening. I was able to successfully clasp push this project previously and didn't get this error then. Did clasp somehow lose support for field declarations recently?

Nu11u5 commented 2 years ago

Did clasp somehow lose support for field declarations recently?

This isn't a clasp issue per-se. The error is returned by the GAS server after it checks the pushed code for syntax errors. You get the same error in the GAS IDE if you try to manually create and save a file containing a class with field declarations. I encountered the same issue in one of my last projects and had to use the constructor to define the fields instead.

@Qythyx I can't say if this is a recent problem or one you just managed to avoid before. For clasp to work around this the transpiler would have to omit any class field declarations from the pushed code (and move any assignments to the constructor). From a functionality standpoint, they are unnecessary in JavaScript.

Qythyx commented 2 years ago

@Nu11u5, I noticed if I look at my transpiled .gs files already pushed to my project, they have a comment at the top that says // Compiled using ts2gas 3.6.4 (TypeScript 4.2.3). I don't remember the last time I pushed, but clearly it was with an older version of clasp and therefore ts2gas.

I tried installing the old clasp v2.2.0 (ts2gas 3.6.5), and with that I could push successfully. I then tried clasp 2.3.1 (ts2gas 3.6.5), but that failed with the same error. I find that odd since ts2gas is the same version in both.

Anyway, at least I have a workaround for now, use clasp 2.2.0. I don't know if you're the right person to work on this, but if you could help figure out why this broke between that version and recent versions that would be great.

Thanks for all your help.

Qythyx commented 2 years ago

Actually, it wasn't that simple. The push succeeded, but that was because it wasn't including all my files (those in subfolders). When I moved things around v2.2.0 failed with a similar error, but v2.1.0 succeeded. Unfortunately it then had errors when I ran my scripts in my spreadsheet.

I then tried checking out various versions of clasp to try to find whatever old version I used to use that allowed me to push successfully. I found that a560632 was when the ts2gas was updated to v3.6.4. That was released in clasp 2.3.1, but that didn't build without me making a few minor changes. After that, though, it would not clasp push successfully with the same error.

After a bunch of trial and error I finally found that tag 2.5.0 works. I had to make one small tweak to the clasp code to get it to run, but after that I was able to clasp push successfully and run my scripts. This version is using ts2gas 3.6.3.

I still don't know what version of clasp I used previously.

Nu11u5 commented 2 years ago

@Qythyx if you look at the code uploaded into GAS using the working version of clasp, how has ts2gas handled class field declarations?

Qythyx commented 2 years ago

@Nu11u5, it basically coverts them to this.field. Here's the transpiled version of Beer.ts:

// Compiled using ts2gas 3.6.3 (TypeScript 3.9.7)
var exports = exports || {};
var module = module || { exports: exports };
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.Beer = void 0;
//import { Document } from "./Document";
class Beer {
    constructor(values) {
        this.id = undefined;
        this.PartitionKey = "Beer";
        this.Details = { English: {}, Japanese: {} };
        this.UntappdDetails = {};
        const asAny = this;
        for (var i = 0; i < Beer.CoreHeaders.length; i++) {
            asAny[Beer.CoreHeaders[i]] = values[i];
        }
        let offset = Beer.CoreHeaders.length;
        for (var i = 0; i < Beer.DetailsHeaders.length; i++) {
            asAny.Details.English[Beer.DetailsHeaders[i]] = values[i + offset];
        }
        offset += Beer.DetailsHeaders.length;
        for (var i = 0; i < Beer.DetailsHeaders.length; i++) {
            asAny.Details.Japanese[Beer.DetailsHeaders[i]] = values[i + offset];
        }
        offset += Beer.DetailsHeaders.length;
        for (var i = 0; i < Beer.UntappdHeaders.length; i++) {
            asAny.UntappdDetails[Beer.UntappdHeaders[i]] =
                (Beer.UntappdHeaders[i] === "LastUpdated")
                    ? Date.parse(values[i + offset]) || new Date(0)
                    : Number.parseFloat(values[i + offset]) || 0;
        }
        this.id = this.id || undefined;
    }
    static Clean(beer) {
        return new Beer(Beer.ToArray(beer));
    }
    static get CoreHeaders() {
        return ["id", "ABV", "IBU", "ImageUrl"];
    }
    static get DetailsHeaders() {
        return ["Name", "Description", "Brewery", "Tags", "Style", "Container", "Size"];
    }
    static get UntappdHeaders() {
        return ["BeerID", "GlobalRating", "RatingCount", "LastUpdated"];
    }
    static get Headers() {
        return ["id", "ABV", "IBU", "ImageUrl", ...Beer.DetailsHeaders.map(h => "English " + h), ...Beer.DetailsHeaders.map(h => "Japanese " + h), ...this.UntappdHeaders];
    }
    static GetValues(beers) {
        return [
            Beer.Headers,
            ...beers.map(beer => Beer.ToArray(beer))
        ];
    }
    static ToArray(beer) {
        const asAny = beer;
        return [
            ...Beer.CoreHeaders.map(h => asAny[h]),
            ...Beer.DetailsHeaders.map(h => asAny.Details["English"][h]),
            ...Beer.DetailsHeaders.map(h => asAny.Details["Japanese"][h]),
            ...Beer.UntappdHeaders.map(h => h === "LastUpdated" ? new Date(asAny.UntappdDetails[h]) : asAny.UntappdDetails[h])
        ];
    }
    ToCreateMessage() {
        return { Beer: this };
    }
    get AsString() {
        return `${this.Details["English"].Brewery} - ${this.Details["English"].Name}`;
    }
}
exports.Beer = Beer;
Nu11u5 commented 2 years ago

@Nu11u5, it basically coverts them to this.field. Here's the transpiled version of Beer.ts:

So in that version ts2gas it is wrapping the field assignments in the constructor, which GAS allows. At some point the transpiler tried to keep them as class field declarations in the class definition, which broke your code.

Nu11u5 commented 2 years ago

@Qythyx I fixed the issue in your sample code by targeting "ES2022" instead of "ESNext" in your tsconfig.json file.

Qythyx commented 2 years ago

@Nu11u5, thanks, that seemed to work. I'm wondering if it is because a change in Typescript's useDefineForClassFields. As of target=ES2022 the default value is now true where it used to be false.

Also, one minor thing. I noticed after the clasp push succeeded the new transpiled Javascript cade has this comment at the top of each file // Compiled using spreadsheetfunctions 1.0.0 (TypeScript 4.5.5). I think it's interesting it says it was compiled via spreadsheetfunctions which is my project, and no longer via ts2gas.

Qythyx commented 2 years ago

Ack, now I'm having this issue with a different project. I tried setting target=ESNext and ES2022, and both have an error. I also tried setting useDefineForClassFields to both true and false. When it is false I get Syntax error: SyntaxError: Unexpected token '{' line: 15 file: Plugins/Linker.gs and when true I get Syntax error: ParseError: Unexpected token = line: 15 file: Plugins/Linker.gs; just a different token.

I'll try to compare to my other project to see if I can figure out why this one is failing, but it would be great if clasp or the underlying ts2gas could be improved to make this work better.

Qythyx commented 2 years ago

Aha! If I change it to "target": "ES2019" then it works. I think that might be the key. Also, looking at the clasp typescript docs it says you should target ES2019. Doh, RTFM, 🤦.

NWPoul commented 8 months ago

Also got problem with class field assignments using third-party npm lib (so cant control source code) target settings in tsconfig obviously doesn;'t help here

solution found - use babel plugin @babel/plugin-transform-class-properties with { "loose": true } option: //babel.config.js plugins: [ [require("@babel/plugin-transform-runtime").default], [require("@babel/plugin-transform-class-properties"), { "loose": true }], ],