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

Do not rename .gs files to .js before pushing #51

Closed
JeanRemiDelteil opened this issue Feb 26, 2018 · 18 comments
Closed

Do not rename .gs files to .js before pushing #51

JeanRemiDelteil opened this issue Feb 26, 2018 · 18 comments

Comments

@JeanRemiDelteil
Copy link
Contributor

Expected Behavior

When using : clasp push
a file named: Code.gs is still named Code.gs (locally, i'm not talking about remote)

Actual Behavior

When using : clasp push
a file named: Code.gs is renamed to Code.js

The project files should not be changed by the pushing operation.
Any preparation to the push operation should be done by the tool, without affecting project files.

Steps to Reproduce the Problem

  1. use clasp push with *.gs files

Specifications

  • Version (npm list | grep clasp): 1.1.5
  • OS (Mac/Linux/Windows): all ?
@grant
Copy link
Contributor

grant commented Feb 26, 2018

@JeanRemiDelteil Is there a specific reason you want to use the .gs extension? The .js conversion is a feature of the CLI.

@JeanRemiDelteil
Copy link
Contributor Author

JeanRemiDelteil commented Feb 26, 2018

Historically *.gs files are Google appsScript server side javascript.
There are various reason to keep the 'gs' part (essentially it comes back to differentiate with normal JS files):

  • AppsScript is actually javascript 1.5 with some feature taken from ES6 (destructuration for example)
  • We can have *.js files in our local project that will be embedded as *.js.html files later for client side javascript, in the same project.
  • server side javascript and client javascript really do not behave in the same way, it's clearer to keep the gs extension.

Appart from these reasons, having the CLI changing files for no real reasons is not a good practice if it's not an intended feature (like : "clasp renametoJS" could do).

@arjun-rao
Copy link
Contributor

arjun-rao commented Feb 26, 2018 via email

@JeanRemiDelteil
Copy link
Contributor Author

JeanRemiDelteil commented Feb 26, 2018

All standard IDE support aliases for javascript files so that is not much of an issue.

But the main issue is that the renaming is done when you upload the files to the remote AppsScript project, not when you are working on it.
It should not be the decision of the tool to change the files name. If you want *.js extension, just rename your file manually in your local project.

We are working with lots of different files and projects, and really appreciate having the clear difference between *.gs et *.js

@grant
Copy link
Contributor

grant commented Feb 26, 2018

I can understand why keeping *.gs is attractive. For most users using *.js is fine or even preferred. In reply to the reasons:

  • Apps Script is ES 1.5 with some global variables
    • It's still js though, just not modern ES7+.
  • *.js files embedded
    • Not sure what this means. Perhaps use a folder for these files?
  • Server/client js
    • Node is still js

.gs is also not recognized by many tools (linters, GitHub search)

Using .js should be the default and is important for future language and tooling development.

If you want to add a .clasp.json setting fileExtension to ProjectSettings that allows you to set the file extension (js / gs), let me know.

Using multiple extensions is a pain as can be seen with react .js and .jsx (Discussion)

@JeanRemiDelteil
Copy link
Contributor Author

I'm still pondering the use of gs / js.
I don't have a public complete gas public project to discuss as a base, but here is a library we made (https://github.com/RomainVialard/FirebaseApp), for gas projects.
This is not meant to work with non-gas project, the extension makes this clear.

Also about the client javascript, we currently have this sort of typical structure (locally):

─ main.gs
─ someLib.gs
─ sidebar/
└─ index.html
└─ clientCode.js.html
─ dialog/
└─ index.html
└─ clientCode.js.html

So you can't really regroup those files in a folder.

I'm also wondering why would the "clasp push" command change the name of my files at all ?

@grant
Copy link
Contributor

grant commented Mar 7, 2018

js is important for the future of Apps Script.
It may make sense to use gs now, but things are changing in the Apps Script ecosystem that will make gs block integrations with libraries that use js.

@joshmreesjones
Copy link

joshmreesjones commented Mar 17, 2018

It seems that Google Apps Scripts only recognizes .gs script files, and not .js script files. When I push a .js file, Google Apps Scripts doesn't show it in the project.

This seems like a serious enough issue to address now because when I push my addon to Google Apps Scripts my code doesn't get there. Even if it were an option in .clasp.json as you mentioned that would be great!

@erickoledadevrel
Copy link
Contributor

+1 to this issue, clasp shouldn't rename files. The clasp push command should be a read-only operation in regards to the file system.

I'd rather see:

  • clasp push supports both .js and .gs extensions, but outputs a warning for .gs files noting the preference for .js
  • clasp pull always creates .js files, regardless of whether or not .gs files are present

@grant
Copy link
Contributor

grant commented Apr 9, 2018

@erickoledadevrel After clasp pull in your example, would you want to see this?

test/foo.gs
test/foo.js
index.gs
index.js

This is what would happen to all of my projects after I first implemented the above bullets.


I too believe that clasp as a tool shouldn't rename files.
But I also believe that we'll need to support tools for local development (TypeScript, linting, execution, library systems). Those features aren't supported today, but will be in the future.

This issue also gets a lot more complicated when introducing ts files.

I support optional file extension support, #51 (comment), or any proposal that supports js and gs.

@JeanRemiDelteil
Copy link
Contributor Author

@grant After clasp pull, existing files should keep their extensions (*.gs, *.js, *.gs.js), new one should have the *.js extension.

One point that is really important here, is that a PUSH operation on the server should not alter the local file system.
You can add another command in the roster to specifically convert extensions, but the PUSH command should only do what its name suggests.

Main issue with this is that we (and certainly others) are using CLASP to manage the pushing to GAS, (never really PULL by the way, as it's not needed after original old project import), but we have other tools in the pipeline, that takes care of the rest.
CLASP CLI --> manage GAS connection.
the rest of our tools --> manage the more specific stuff we need to have.
GIT --> manage versioning (and it does not like name change at random).

@erickoledadevrel
Copy link
Contributor

@grant - Yes, I'm fine with that as a result. As long as we warn the developer, they can deal with the repercussions. Trying to match up existing .gs files and preserve the extension might be nice, but I think it's unnecessary.

To your .ts point, I don't think we need to support two-way sync for the professional developer. How often does an AppEngine developer pull down their production code? How often do people pull down production Heroku code? I think the professional developer will be OK treating Apps Script as an environment you only push to. clasp pull is a nice feature for folks trying to migrate existing scripts to a local-development model, but I don't think we should over-fit for it.

@iepoch
Copy link

iepoch commented May 31, 2018

This is also an issue when using gulp. If it renames .gs to .js and I ignore the .js file it will not upload. But if I don't ignore the .js file the gulp file gets renamed also. Which causes more issues to run gulp and have it watch and update the file. Will there be fix for using the just the .gs files without it renameing or is there a way to stop the renaming. I just started using this and need it to create gas files.

@erickoledadevrel
Copy link
Contributor

What about adding a field to the .clasp.json file that specifies the file extension to use. clasp will only consider that extension for uploading "code" files, and will use that extension when syncing files from Apps Script. It would default to "js", but developers can set it to "gs" if they would prefer to stick with that extension.

@grant
Copy link
Contributor

grant commented May 31, 2018

Implementing #51 (comment) seems like a good choice.
It'll be pretty rare where we intentionally push a project with NAME.js and NAME.gs.

After this discussion, following Eric's comment I'll implement this:

  • Delete the renaming files on pull. Always create js even if gs is present.
  • Add a warning when push conflicts occur.

@erickoledadevrel Last thing, after the push warning, do we want to proceed to push js or gs, or err/abort instead of warn?

@DimuDesigns
Copy link

DimuDesigns commented Jun 15, 2018

This "feature" has become something of a pain point for me and has tempered much of my initial enthusiasm for this tool.

I made a suggestion before (#27) similar to @erickoledadevrel. I'll reiterate it here. It may be a good idea to allow developers to opt-in or opt-out of this feature on a per project basis by allowing them to set a config option. Say:

clasp config --rename_locally_to_js true

Where *.gs files are NOT renamed by default, the user has to explicitly set that config option.

@grant
Copy link
Contributor

grant commented Jun 15, 2018

@DimuDesigns @JeanRemiDelteil I've removed this feature that renames gs to js on clasp push (PR #27). Please download the latest release of clasp and try it out to ensure it works for you.

In the future we can support custom file extensions with clasp. Currently, as @erickoledadevrel stated above with #51 (comment) (and I totally agree with), clasp should not implicitly change the local filesystem. I pardon the delay with fixing this issue.

@JeanRemiDelteil
Copy link
Contributor Author

much appreciated 👍
I suppose we can close this issue now ?

fixed by #27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants