Giter VIP home page Giter VIP logo

Comments (36)

DimuDesigns avatar DimuDesigns commented on June 14, 2024 2

Best practice aside, sometimes there is a need to store configuration options (some generated from custom classes) as global constants/enums. I could wrap said properties into getter/accessor functions or create some kind of singleton composed of such functions but that feels superfluous since I know I can rely on execution order. Unfortunately, clasp breaks that reliability.

@JeanRemiDelteil Following a clasp push the script files are re-ordered to execute alphabetically (in truth lexicographical-ly) instead of the order in which they were created. However, that ordering is not reflected visually in the Apps Script editor so I don't see the value in retaining this weird ordering glitch. But there is an option to sort files alphabetically under the view menu in the Apps Script editor.

from clasp.

JeanRemiDelteil avatar JeanRemiDelteil commented on June 14, 2024 2

The question of running script in parallel in totally different from the matter of file execution order.

In AppsScript, strictly speaking, real parallelism or asynchronous behavior like you would expect with Promise, timeOut or thread does not exist at the moment.

All files are read and executed in their creation order, which is why using global is not recommended.
This creation order was historically the order of file creation in the web editor.

And only for standalone script (not container bound script, eg: script linked to a spreadsheet and such), you could get and set the content as a Drive file.
In such a case, execution order was the order in which files were in the drive file (with GAPPS, that was alphabetical order).

The new GAS API is totally different in its disclosed manner of action, in the way that it is a API working on AppsScript projects, and not on Drive files. As such it is not bound the standalone restriction, it can work on standalone and container bound both (which is a GREAT improvement, waited for many years).

Relying in wacky hacks due to Script execution order should not be the way in AppsScript.
There are way better ways to achieve initialization with better control.

For example, you should know were all your entry points are located, and at the beginning of all those functions, just call the function initializing everything (by returning an object).
Just to mention, in AppsScript, the 'this' object is the global object.

I have an example of this in this simple project: monitor-firebase-status (and I would be happy to discuss over this method of initialization).

from clasp.

grant avatar grant commented on June 14, 2024

Thanks for the detailed report.
I will address this issue in the next major release.

As a workaround, encapsulate all statements in functions rather than global scope as a best practice to avoid execution order dependencies.

from clasp.

JeanRemiDelteil avatar JeanRemiDelteil commented on June 14, 2024

@DimuDesigns the previous command line tool also had this behavior (gapps)
And it's pretty normal to not rely on something as random as creation order of the files.

As grant underlined, you should not have any global executing code. It's advised to know exactly where are your entry points.
Using global scope may cause unexpected variables overwriting and such.

@grant it would be great to also keep the alphabetical ordering capability

from clasp.

JeanRemiDelteil avatar JeanRemiDelteil commented on June 14, 2024

For visual ordering, you may want to try my AppsScriptColor extension that will display the files always alphabetically, and with folders if you name your files with '/' inside (works well with CLASP).

from clasp.

grant avatar grant commented on June 14, 2024

Technically, I believe execution order is based on file creation date within the IDE.
In the API, the order of the files in updateContent. It causes lots of bugs for users, so we generally recommend putting all code in functions.

from clasp.

DimuDesigns avatar DimuDesigns commented on June 14, 2024

@grant Using the old import/export method that existed prior to the apps script api (documented here) seems to retain the correct execution order. Maybe you could incorporate those methods to make clasp more robust.

Or (using the new api) before calling updateContent do a getContent to work out the correct order and then push based on that.

EDIT
Noticed that the old import/export method had ids for each of the files in the project. However the new apps script api does not when getContent is called. Maybe your team can bring the two methods (import/export and the new api) more inline to smooth out these issues.

from clasp.

rafasc avatar rafasc commented on June 14, 2024

Is execution order well defined behaviour or were people using it just because it works?

What should happen if we modify the order files are sorted? View -> Sort Alphabetically (And possibly new ones in the future)?
Should it also modify execution order? What if I need a certain execution order, but enjoy viewing files in a totally different order?
What about asynchronous "order"? Is there a reason scripts can't be run in parallel? 😃

If this order issue is really a thing that can't be easily fixed by creating functions, I would rather have the option to edit the order explicitly in the manifest file than relying on naming conventions.

from clasp.

DimuDesigns avatar DimuDesigns commented on June 14, 2024

@rafasc I can't speak for other developers, but in my experience, sorting the files alphabetically within a project via View -> Sort does not impact execution order. From what I can tell execution order is equivalent to the order in which the files are created within a project which appears to be independent of view order.

As for running scripts in parallel (asynchronously), there are folks in the apps script community who have managed to achieve this to a degree (linked here). But I suspect that, in terms of granularity, parallelism is only achievable at the session level, ie. multiple instances of a project are spun up (say via google.script.run from the client-side) and run/execute concurrently. Whereas individual script files/functions within a project probably cannot be made to do so within the current session (but I'm guessing here).

from clasp.

DimuDesigns avatar DimuDesigns commented on June 14, 2024

@grant I just realized that this issue also highlights another oddity. If you look at the timestamps in the logs (screenshots 4 and 6) the timezone changed following the call to clasp push. It was EDT before and now its PDT after. I checked the Project Properties -> Info and the timezone is set to Eastern Time as expected but the log timezones are now PDT. Weird...

from clasp.

Nebucatnetzer avatar Nebucatnetzer commented on June 14, 2024

What is the status of this bug?
I just ran into this problem with a third party library and it took me quite some time to figure out what went wrong.

from clasp.

grant avatar grant commented on June 14, 2024

I still need to add the createTime and updateTime of a File here:
https://github.com/google/clasp/blob/master/src/files.ts#L123

Hopefully that would fix this bug.

from clasp.

Nebucatnetzer avatar Nebucatnetzer commented on June 14, 2024

That would be great. Otherwise I probably have to rewrite the library.

from clasp.

grant avatar grant commented on June 14, 2024

Btw, this is the bug for reordering script in the script editor: https://issuetracker.google.com/issues/36756144

from clasp.

grant avatar grant commented on June 14, 2024

I've spent a few hours trying to solve this issue. This is what I've done:

  • Look at how to set the creation/modified date of a unix file.
    • A: It seems impossible, you can only touch a file to set the modification date to now.
  • Look how gapps may have done this.
    • A: It looks like gapps didn't modify the creation date. Not sure how this was solved in that tool.
  • Look how import-export may have done this:
    • It looks like there's simply an array of files that you can put in or out. I guess if you manually stitched the files correctly, it would work, but that's a bit manual.

The solution seems to be this:

  • clasp pull: Does a unix touch in the order of the file.
    • Delays each fs.writeFile with a setTimeout(() => {}, 1);
  • clasp push: Reads all files and does a fs.statSync to organize the array of files before push.

from clasp.

JeanRemiDelteil avatar JeanRemiDelteil commented on June 14, 2024

@grant : I'm sure there are reason to fix this 'bug' however keeping the alphabetical ordering as an option would make sense.
(Though i'm not to stuck up on this, as for all I've said above the file order does not really matter, and my extension and the original UI allows to show the files in order !)

from clasp.

grant avatar grant commented on June 14, 2024

If alphabetical ordering (_.gs, a.gs, b/z.gs, x.gs) would fix everyone's issues then I'd gladly implement that and update clasp this week. I hate that this large issue hasn't been solved since March and affects some claspers.

Is there anyone opposed here to that?
@DimuDesigns @rafasc @Nebucatnetzer

from clasp.

Nebucatnetzer avatar Nebucatnetzer commented on June 14, 2024

I'm not sure I understand it.
Isn't alphabetical ordering the way it currently works and that is causing problems?
At least my workaround for the problem was to name the files Z01_xxx, Z02_xxx, etc.
So that they keep their order.

from clasp.

JeanRemiDelteil avatar JeanRemiDelteil commented on June 14, 2024

Yes i'm pretty sure it's how the current implementation works.

It's just something you need to take in account when coding.
Alphabetical order is so easy to understand instead of invisible shaddy creation dates imho !
It is also reproducible on all system and fit for a streamlined work process (as the result will always be the same).

from clasp.

Nebucatnetzer avatar Nebucatnetzer commented on June 14, 2024

I personally can live with that, however then it is more a problem for the Google Apps Script Browser UI.
Because there the scripts are shown in the order they get created and not in alphabetical order.
I don't know if @grant can do something about that.

from clasp.

DimuDesigns avatar DimuDesigns commented on June 14, 2024

Look how import-export may have done this:

  • It looks like there's simply an array of files that you can put in or out. I guess if you manually stitched >the files correctly, it would work, but that's a bit manual.

I've found that the arrays retrieved using import-export actually retain the execution order of the script files in an app script project (as seen in the IDE).

I don't like the idea of a tool automatically changing underlying structures that can adversely affect my code. So if at all possible please retain the execution order.

from clasp.

JeanRemiDelteil avatar JeanRemiDelteil commented on June 14, 2024

@Nebucatnetzer It's possible to show the scripts in alphabetical order in the Google Apps Script Browser UI.
You have to check a menu:
image

from clasp.

Nebucatnetzer avatar Nebucatnetzer commented on June 14, 2024

Ah okay, but that doesn't change the execution order?

from clasp.

JeanRemiDelteil avatar JeanRemiDelteil commented on June 14, 2024

No. It's display only. (Which is why alphabetical order execution in this case makes sense)

I myself don't write any code in the browser UI any more, even the files are created on my desktop, then pushed to the script when testing, so using the GUI is anecdotal..

from clasp.

Nebucatnetzer avatar Nebucatnetzer commented on June 14, 2024

Ah okay, I agree that alphabetical execution order makes the most sense.
At least it's something which is easy to understand/lookup.

However that still breaks existing projects which rely on the date execution order.

from clasp.

grant avatar grant commented on June 14, 2024

EDIT: This plan doesn't work because filesystems don't store original creation times after cloning scripts.

It seems like preserving existing execution order (which is based on the create date for each file you've created) is the only solution for preserving old script execution order.

I'll eventually add this feature, but it's going to take some time and testing since filesystems don't have a way to manually override file creation time (ctime). Here would be the implementation plan.

WRITE to FS (after clasp pull):

READ from FS (clasp push):

  • For every non-ignored file:

This would preserve file creation order in and out of the script.google.com UI.

from clasp.

JeanRemiDelteil avatar JeanRemiDelteil commented on June 14, 2024

Ok, Just a question on this.

We are only using READ from FS. never Write, or juste to init a project from existing really old script.
With your plan, you will order files against a creation time that came from just creating files when developing the project on the FS.
Those files are stored in a version control system, that can delete and create them at will, hence changing randomly the creation date.

As this makes no sense, but needs to be solved, I propose that you store the existing files order when pulling them, in the .clasp.json settings file.
Then use this order first when pushing, and add all non-mentioned files in alphabetical order (and do not update the .clasp.json with them unless pull is used again).

This method should solve this issue and also provide an easy / simple / consistent way to keep existing file ordering or manually / programmaticaly temper with file ordering !

from clasp.

grant avatar grant commented on June 14, 2024

I agree that deleting/creating/moving the files will alter the order. Even copying the files from one folder to another would break the order. I'm open to any long-term solution, but I still don't see one.

My initial reply would be: Storing the push order of files doesn't work because you may rename/move the file.

However, a push order may be the best and most accurate way to push files in order.

Here's an idea: I could create a list of files that clasp should push first, but that doesn't seem scalable.

export interface ProjectSettings {
  scriptId: string;
  rootDir: string;
  projectId: string;
  filePushOrder: string[];
}

In this example, you could define filenames in your .clasp.json. It's up to you to ensure that list is up-to-date.

Would this work for everyone?

from clasp.

Nebucatnetzer avatar Nebucatnetzer commented on June 14, 2024

For me this would be fine.

from clasp.

JeanRemiDelteil avatar JeanRemiDelteil commented on June 14, 2024

@grant : and if there are files not mentioned in this list, I expect them to be pushed alphabetically ordered.
That would be ok!

About scalable:
The whole 'creation time' ordering is not scalable from the start. There are no serious project/solutions out there that use this kind of behaviour.
So it's not a real problem: the issue here is to prevent old project from breaking when pulling and pushing again.

About file being referenced by name: again, that is not a real issue ! If a user changes a filename (which is the only ID of the file), then he will probably make other changes as well and he know what he is doing.

from clasp.

grant avatar grant commented on June 14, 2024

@DimuDesigns @JeanRemiDelteil @Nebucatnetzer @rafasc
Can you install [email protected] and use the new filePushOrder setting?

It should work like this:

clasp status
Detected filePushOrder setting. Pushing these files first:
└─ main.js

Not ignored files:
└─ main.js
└─ 1.js
└─ 2.js
└─ appsscript.json
└─ fstest.js

Ignored files:
└─ .clasp.json
cat .clasp.json
{
  "scriptId":"1pbtnbRjwdPOAUdmTViOZnWSrqoYPa3Y3xWFzAxOZh1HvesRRKiLu7S3j",
  "filePushOrder":["main.js"]
}

Assuming the API is not buggy, it sends the files in an array as listed in the "Not ignored files" list.

from clasp.

Nebucatnetzer avatar Nebucatnetzer commented on June 14, 2024

Thanks I'll check it out next week.

from clasp.

grant avatar grant commented on June 14, 2024

I'm closing this issue as I think it's fixed with the new filePushOrder feature. If it's still broken, we can re-open the issue.

from clasp.

JeanRemiDelteil avatar JeanRemiDelteil commented on June 14, 2024

@grant the documentation mention

All other files are pushed after this list of files.

Maybe you can detail in which order ? (alphabetical I suppose ?)

All other files are pushed in alphabetical order after this list of files.

I find this important as it's the default behavior.

from clasp.

JeanRemiDelteil avatar JeanRemiDelteil commented on June 14, 2024

@grant I just tested, the "filePushOrder" PR reopen issue #48

Meaning my folder/file architecture is named with a "\" instead of a "/"
This breaks all the HTML imports based on file name.

The final file names should be consistent even when pushed from different OS (window / linux / mac)

from clasp.

valen22br avatar valen22br commented on June 14, 2024

@grant I just tested, the "filePushOrder" PR reopen issue #48

Meaning my folder/file architecture is named with a "" instead of a "/"
This breaks all the HTML imports based on file name.

The final file names should be consistent even when pushed from different OS (window / linux / mac)

@grant, I'm having the same problem here regarding HTML file import.
Any idea how to solve the problem?

from clasp.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.