Skip to content

feat/allowjs-in-project #1261

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

Closed
wants to merge 7 commits into from
Closed

feat/allowjs-in-project #1261

wants to merge 7 commits into from

Conversation

rwatts3
Copy link

@rwatts3 rwatts3 commented May 18, 2017

  • All compiled assets are included (atom packages are git tags and hence the built files need to be a part of the source control)

First attempt/iteration to get typescript support in js files.
With these updates, you would have to switch the grammar of a js file to ts.
ctrl+shift+l

@rwatts3
Copy link
Author

rwatts3 commented May 18, 2017

Also please note this is just for the autocomplete/intellisense.
I haven't gone as far as allowing typescript to compile the js files etc. I don't think this is currently possible with typescript. But at least we'll be able to get some form of autocomplete in js files.

@@ -119,7 +119,7 @@ export function getRangeForTextSpan(editor: AtomCore.IEditor, ts: { start: numbe
export function getTypeScriptEditorsWithPaths() {
return atom.workspace.getTextEditors()
.filter(editor=> !!editor.getPath())
.filter(editor=> (path.extname(editor.getPath()) === '.ts'));
.filter(editor=> (path.extname(editor.getPath()) === '.ts' || '.js');
Copy link

@zaaack zaaack Jun 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean path.extname(editor.getPath()) in {'.ts': 1, '.js': 1} ?

@guncha
Copy link
Contributor

guncha commented Jun 24, 2017

Now this is interesting. With minor changes (pushed here in check-js-support) and when used in a project with checkJs enabled, I could get full Typescript support in JS files, including autocomplete, diagnostics, compile on save and the usual commands like rename refactor, etc. Suddenly we have a half-decent javascript IDE!

I'd like to release this behind a feature flag in the settings to let people play with it and get us some feedback since this is clearly experimental. @rwatts3, do you mind doing the work?

@rwatts3
Copy link
Author

rwatts3 commented Jun 25, 2017

Yes i'd be happy to see what I can do.
If you want to create an issue or what would be the best place for me to have sort of a checklist , probably a separate PR ?

I will start with cloning the check-js-support branch.

I would also like to say that this is probably the single most important addition to Atom that could potentially win over a lot of people, especially those who enjoy Atom, and are constantly forced to use Webstorm and/or VSC for intellisense purposes.

@guncha
Copy link
Contributor

guncha commented Jun 25, 2017

You can just merge the check-js-support branch into the branch of this PR (perhaps also enable Allow edits from maintainers as described in the link below so I can push commits if necessary) and continue to work on it here.

To break it down, we need:

  • A new boolean config added to the config export in atomts.ts that says something like "Enable Typescript support in .js files" with a note "This requires enabled "checkJs" option in the projects "tsconfig.json". This should default to false.
  • Update the helper isTypescriptGrammar in typescriptEditorPane.ts to take this setting into account
  • Update helpers in utils/atom.ts to take these into account
  • Keep the AutocompleteProvider's selector the same, but return early from getSuggestions if the enableJs config is false.

For the TypescriptEditorPane class, we'll also need to watch for changes to the config and react accordingly, but that can be done later.

P.S. Forgot to add the link - https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork/

@rwatts3
Copy link
Author

rwatts3 commented Jun 27, 2017

@guncha I may need some help with the other items, I'm still fairly new to atom packaging.
I've added the option for checkJs, however i'm not sure how to complete the others you specified above.
Also we should include the option for when there is a html file, that contains a script tag with a source of .js .

@guncha
Copy link
Contributor

guncha commented Jun 27, 2017

Yeah, I have some ideas how to update the helpers to take into account the allowJs setting. As for supporting HTML files, that's a whole another can of worms that Typescript doesn't support out of the box, but it can be implemented using the plugin support they recently added. The short version is that you have to trick tsserver into thinking that every single script tag is its own file and then translate the file positions back and forth when issuing commands. It can get hairy. If some brave soul implements a plugin for TS support in HTML files, it would just work with this JS support that you're working on.

@rwatts3
Copy link
Author

rwatts3 commented Jul 3, 2017

just wanted to follow-up, I won't be able to get to the other pieces, for this in case someone else want's to chime in and help wrap this up.

@zaaack
Copy link

zaaack commented Jul 4, 2017

Thanks, you guys, this PR will help me a lot.

@rwatts3
Copy link
Author

rwatts3 commented Jul 6, 2017

@guncha are you guys going to be able to finish this out and complete the PR ?

@cancerberoSgx
Copy link

Please, please, support allowJs !! Lots of us, must keep plain-old-javascript and thanks to typescript - jsdoc and google closure now we are able to do type-checking in plain-old javascript :) This adds lots of value currently I'm (and my users) are forced to switch to visual-code-studio or webstorm that do support this.

@brettz9
Copy link

brettz9 commented Jul 2, 2018

There are millions of people waiting for this to be rebased and land. No pressure. :-)

@lierdakil lierdakil mentioned this pull request Nov 3, 2018
10 tasks
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

Successfully merging this pull request may close these issues.

5 participants