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

An alternative attempt at getting modules to work #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chrisprice
Copy link

I had a play around with the original version but didn't get very far because I didn't understand the internals of require.js or the JavaScript language service. I figured the best way to learn was by trying to re-write it myself and so here it is.

I doubt you'll want to accept the pull request as it is (it's a complete re-write after all) but hopefully it can provide some inspiration.

If you want to see it in the context of the application I was writing then checkout my PropertyFinder-HTML5 fork (iPhonePropertySearch.sln), and if you want to read more about it then there's a Code Project article.

I'm really interested in making the most out of these latest Intellisense features, JavaScript desperately needs better tooling!

Cheers,

Chris

@jrburke
Copy link
Owner

jrburke commented Sep 18, 2012

Thanks for taking a shot at this. I do want anonymous modules to work though -- they are best practice, and I expect will be the majority of modules. I tried an exploration too in this pull request:

#5

but I could not get it to work. If you have any insight, it is appreciated.

I think we just need to get more details on how visual studio is doing the script work under the covers.

@jmatthiesen
Copy link

Hey there - I'm a program manager at Microsoft working on the IntelliSense experience for JavaScript and it would be great to see the require.intellisense.js extension working. Any questions I can help answer about what's going on "under the covers?" - Jordan

@chrisprice
Copy link
Author

Hi Jordan, I'm away from the office at the moment so don't have access to a VS install but I'm very interested in helping to get this going. Can I get back to you next week? In the mean time I'd be interested if I was correct in the "What's Happening Behind the Scenes?" section of the linked CodeProject article?

@jrburke
Copy link
Owner

jrburke commented Jan 31, 2013

@jmatthiesen: I tried a pass at this a while ago in pull request #5 -- it has been a while since I looked into it, so I do not have the full context at the moment, but I have some notes in that pull request in the approach I was trying to do. Any insight into the following, that might be a starting place, if @chrisprice cannot dive into it more sooner:

So I was hoping that in each call to define(), I could do a setTimeout to allow completion of the current script, then look for at the added scripts, find the one that matches a path that was recorded via requirejs.load() and if so, trigger onScriptLoad() for that script.

The trouble seems to be that the setTimeout function tracked by loadTimeId is triggered, but there are no scripts with a script src when it fires.

I guessing there is some magic that intellisense is doing, maybe I'm triggering that loadTimeId setTimeout at the wrong time.

Of course I could just be taking a completely wrong approach with that pull request too.

@chrisprice
Copy link
Author

Hi @jmatthiesen,

I've just been talking this through with my colleagues who've been facing a similar problem. The crux of the problem is that as IntelliSense is reloading the referenced files, there is no way for the running code to know which file is currently being executed. E.g. -

If A loads B via a script tag, then my understanding is that IntelliSense caches that A depends on B and will load B before A, the next time IntelliSense is invoked from A.

During this execution there is no way for the code to know that B is the file that is being re-executed, without this knowledge the AMD loaded can't properly associate the code with it's alias.

Is there any chance of adding a property such as intellisense.filename which wouldn't be a string containing the path to the currently executing file (n.b. not necessarily the file in the editor).

Hopefully that makes sense! If not feel free to email me [email protected] or we can talk it through on the phone.

Cheers,

Chris

@jmatthiesen
Copy link

Hey Chris,

Thanks for the details! We're talking about it in the team and I'll follow-up with some more questions as we have them.

Jordan
Program Manager, Microsoft Visual Studio

@steeleprice
Copy link

Chris and Jordan,

Can I help with this? I am actively trying to get better Intellisense out of my Require Modules and work with it in VS2012 daily. I do have Require Intellisense working but not modules due to the above comments by Chris.

Steele Price
Microsoft MVP

@jmatthiesen
Copy link

Hi all!

An update for you on this - we have been working on adding RequireJS support to the JavaScript editor in Visual Studio. With the Visual Studio 2013 Update 3 release that just came out, we have two things to help here:

  1. A new property off of the intellisense object called intellisense.executingScriptFileName, as requested here
  2. We have a start on formal requireJS support

We aren't calling the requireJS support final yet, because we have some issues with path resolution and commonJS module syntax to finish up. If you want to try it out as-is, though, here are the steps:

  1. Include RequireJS in your VS project as a file named require.js.
  2. If you're in a .jsproj project file, you can add a .html file with a reference to require.js, data-main configured, etc. and it should kick in
  3. If you're in an ASP.NET web project (Website, Web Application), you can add a /// element to your .JS files or _references.js and this support should kick in

No guarantee it will work for you just yet, but I'd love to know if it does or doesn't work and get some samples from you if it doesn't work. I'll keep you updated!

Jordan
Program Manager, Microsoft Visual Studio

@steeleprice
Copy link

Jordan,

This is excellent news! I will give your recommended techniques a try in the next couple days and report back. I greatly appreciate your work on this. I can talk to you more in depth on this personally in November hopefully.

Steele Price
Microsoft MVP

@gimelfarb
Copy link

@jmatthiesen - Thank you for working on the RequireJS support in VS, I've also been trying last few weeks to get it to work. I was excited to download the Update 3 to try out the new feature, but got stopped dead in my tracks ...

@jrburke , @chrisprice - Sorry for barging in, but seems like an appropriate forum for this.

Looks like previous functionality of JavaScript Intellisense engine was lost in the update. That is the ability to recognize tilde-slash (e.g. '/www/my.js') paths for dynamic scripts. You see, I have a configuration for RequireJS inside _references.js to use '/www/js' as a base URL for resolving module references. It has to be like this, because I want it to be relative to web application root. And it was working before (I had Update 1 installed).

Now with Update 3 this is no longer the case. I create a simple Gist to illustrate - https://gist.github.com/gimelfarb/923fce133d7b81b35780 (that doesn't use RequireJS, just the dynamic script loading capability for simplicity).

test/b.js - when it loads Intellisense context, it will complain that cannot resolve reference for ~/www/js/a.js. Which is being loaded dynamically through load() call, which creates a script tag in the head.

The problem is that I cannot see a workaround:

  • tilde-slash doesn't work
  • beginning slash (e.g. /www/js/a.js) for absolute path doesn't work
  • full filesystem path (e.g. C:\my\path\a.js) works - but not really a solution

I see that a relative path is resolved against currently loading script, which is non-deterministic, and so a relative base path cannot be put into _references.js.

Does the above make sense?

@jmatthiesen
Copy link

@gimelfarb This is a great bug you found, please, can you report this to our Connect site so I can officially track it and keep you updated on progress?

In the meantime, I'll discuss this in my team.
Jordan

@gimelfarb
Copy link

@jmatthiesen - Absolutely, I created a bug report on Connect site. Thanks for the quick response!

@gimelfarb
Copy link

@jmatthiesen - Are there any updates on this? My Connect bug report got a comment that '~' never worked and I should use absolute path. That's incorrect and both no longer work in Update 3. Did you manage to discuss with the team?

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