Community
Participate
Working Groups
Right now as soon as a file is open, it gets added to the tern server through an "addFile" request. This is causing different results for find references or other requests when files are opened in a different order and no .tern-project file is used. In order to prevent this file loading ordering issue, we should limit when a file is added to the tern server. When a .tern-project file is defined, it can have a loadEagerly files property that tells the server what files should be visible. require.js defines a module system that can also define dependencies. These two loading mechanism should use "addFile" requests. Otherwise "addFile" requests should not be used.
Outside of dependencies (requireJS, node, htmlDependencies), where are we calling addFile? Most of our Tern requests will contain the origin file which will perform the same actions as addFile().
*** Bug 488804 has been marked as a duplicate of this bug. ***
1) get minesweeper-demo 2) select index.html 3) refresh the browser with index.html selected 4) click on main() 5) Open Declaration (it goes there) 6) click on setValue() 7) Open Declaration (is goes there) 8) click on main.js 9) refresh the browser with main.js selected 10) click on setValue() 11) Open Declaration 12) BUG: 'could not find declaration' This is a long standing issue with Orion tooling when it is used without RequireJS or another module system. The fact that it is subject to the order in which you select the files can obscure bugs. Could we do this: If you click on a file that does not have a module system and there is no .tern-project file, flush Tern and throw away all information except the information for the file that you just clicked on? If you request an operation that needs Tern to do cross file actions and there is no .tern-project file, we prompt for the file (include a link in the error message or something).
I like this title better: "Without a .tern-project file, Orion tooling behaves differently based on the order that files are selected" versus "addFile ... " implementation detail. As long as the bug is fixed, I am good.
I toyed with making this a P1 but couldn't justify it. We need a solution to this problem because it gives people the impression that our tooling is not working. It's important to me that we get to the bottom of this.
I can add a rule that checks if the ast.environments is empty. If it is, then I can suggest a quickfix to add the current file into the .tern-project file as a loadEagerly file. Like that, if the user applies the quickfix, the .tern-project file contains what the user has already loaded and we can keep a consistent behavior over page reload.
(In reply to Olivier Thomann from comment #6) > I can add a rule that checks if the ast.environments is empty. If it is, > then I can suggest a quickfix to add the current file into the .tern-project > file as a loadEagerly file. > Like that, if the user applies the quickfix, the .tern-project file contains > what the user has already loaded and we can keep a consistent behavior over > page reload. That sounds good to me. Then we accomplish two things at once: 1. we make dev aware of the .tern-project file 2. we help them craft a setup the makes their env / use consistent
Created attachment 260211 [details] Proposed patch Curtis, please review.
I found a bug when the .tern-project file doesn't exist. Will post a new patch shortly.
During the presentation, I had a bug where I cleaned up all errors in the demo code, then renamed main() to main2(), then used 'Show Problems ...' to make sure that the rename did not cause issues. It found problems around main() that were not there. I can enter a bug for this but I wonder if this patch will fix the problem? Of course, when I refreshed, the problem was gone.
Created attachment 260222 [details] Proposed patch This patch properly creates the .tern-project if it doesn't exist yet. But the file doesn't appear in the file explorer (left column). This is not ideal as we say we will add the current file to the .tern-project file. We do, but the file doesn't appear.
(In reply to Steve Northover from comment #10) > During the presentation, I had a bug where I cleaned up all errors in the > demo code, then renamed main() to main2(), then used 'Show Problems ...' to > make sure that the rename did not cause issues. It found problems around > main() that were not there. I can enter a bug for this but I wonder if this > patch will fix the problem? Of course, when I refreshed, the problem was > gone. Please enter a bug. This patch adds a new check and quickfix, it will not affect the workflow you are describing.
The only remaining issue with that patch will be solved once bug 488582 is fixed. There is nothing more I can do from my side for this. I tested the latest changes and everything seems to work ok now.
One thing I cannot get to work today is the fact that I cannot know all dependencies for a file referenced inside the loadEagerly files. For example, if you have a html file that references a bunch of js files, when you open any of these js files, you will still get an annotation that asks you to add that file to the .tern-project file. We would need an API from the tern server to know what are the dependent files.
- Quickfix shouldn't be fixAllEnabled: true - The quickfix adds in a 'projectLoc' entry to the .tern-project file. This is added before validating the file in parseTernJSON, but I don't think it was intended to actually add it to the file. - Opening org.eclipse.orion.client.core/web/orion/Base64.js results in: UncaughtTypeError: Cannot read property 'length' of undefined ... validator.js:250 - The warning message does not give enough information to the user about what the problem is (and it is on by default). The message should explain that they don't have a configured .tern-project file. Would be nice to have a quickfix to set up their environment as an alternative to adding things to loadEagerly. - Warning is annoying in a project full of snippets like smokeTests where I don't want to configure a .tern-project file. I want an option to turn off the checking for this project. Since we don't have project level pref settings, what if we allow an empty .tern-project file to mean 'Don't warn me about Tern configuration issues'. This could also turn off the cross file linting warnings too.
Setting: 'Check if the file should be added to .tern-project' should be 'File should be added to .tern-project' and must be sorted alphabetically by setting string on the page (sorted by rule name on the wiki page).
(In reply to Curtis Windatt from comment #15) > - Quickfix shouldn't be fixAllEnabled: true Fixed > - The quickfix adds in a 'projectLoc' entry to the .tern-project file. This > is added before validating the file in parseTernJSON, but I don't think it > was intended to actually add it to the file. Don't understand what you mean. > - Opening org.eclipse.orion.client.core/web/orion/Base64.js results in: > UncaughtTypeError: Cannot read property 'length' of undefined ... > validator.js:250 Fixed. > - The warning message does not give enough information to the user about > what the problem is (and it is on by default). The message should explain > that they don't have a configured .tern-project file. Would be nice to have > a quickfix to set up their environment as an alternative to adding things to > loadEagerly. We should discuss that. I think the purpose of that is to make it on by default. But we might want multiple ways to fix this. One of them being what I am doing right now. > - Warning is annoying in a project full of snippets like smokeTests where I > don't want to configure a .tern-project file. I want an option to turn off > the checking for this project. Since we don't have project level pref > settings, what if we allow an empty .tern-project file to mean 'Don't warn > me about Tern configuration issues'. This could also turn off the cross > file linting warnings too. Sure. We need a way to turn it off for a project since we don't have project-specific settings. We need to find a moment to discuss this as a team. >Setting: >'Check if the file should be added to .tern-project' >should be >'File should be added to .tern-project' Fixed. >and must be sorted alphabetically by setting string on the page (sorted by >rule name on the wiki page). You mean sorted based on the message (setting string). This would make it locale specific. I fixed it. I need to update the wiki page once this is released.
Since we still need to discuss the right workflow, I would set the default of this new setting to be off. I can deliver what I have so far which is fixing all issues reported by Curtis besides the ones related to the actual workflow.
Created attachment 260229 [details] Updated patch Patch that is fixing issues reported by Curtis.
Created attachment 260231 [details] Last patch This is fixing also the addition of the projectLoc property in the .tern-project file. The new setting is turned off by default.
I delivered my latest patch to master with the setting turned off by default.
When playing with the feature turned on for the demo code (after all errors have been resolved), I have seen "Counter is unused" error come back. I can't get it to happen every time but I have a .tern-project file, I select counter.js and I refresh the page. I will enter a formal bug report for this or you can do this if you can recreate the problem. It seems unrelated to this fix.
I am marking this at P1 which is a bit of a stretch but the reality is that people will not have .tern-project files off the bat and will expect things to just work. It might be that this effort is part of the "sniff for files" thing that Mike and I have talked about.
I pushed in the idea of a 'javascript project' that can allow us to read / update all of the important files we want to 'sniff' for (so far): http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=3b049d2bfdc5cb3cf27f215ee564b45ef1b5f8da So far we have the ability to look in: 1. .eslintrc 2. .tern-project 3. jsconfig.json 4. project.json 5. package.json 6. node_modules now all we have to do is hook it all together in one usable form so if any / all of that info is available we can auto-magically merge it all into a .tern-project file and have things 'just work'.
(In reply to Michael Rennie from comment #24) > I pushed in the idea of a 'javascript project' that can allow us to read / > update all of the important files we want to 'sniff' for (so far): > > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/ > ?id=3b049d2bfdc5cb3cf27f215ee564b45ef1b5f8da > > So far we have the ability to look in: > 1. .eslintrc > 2. .tern-project > 3. jsconfig.json > 4. project.json > 5. package.json > 6. node_modules > > now all we have to do is hook it all together in one usable form so if any / > all of that info is available we can auto-magically merge it all into a > .tern-project file and have things 'just work'. But we should do this only once, right?
(In reply to Olivier Thomann from comment #25) > But we should do this only once, right? yes, ideally we would only do this once. I pushed a bunch of updates: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=742d44fc6d5e64f24bd9f4ed12476c22f8c7e64b that hooks up the jsProject to drive the tern project manager and be used in the quickfixes.
Steve, after discussing with Michael, we cannot go in the direction of not using the tern server at all. We can try to minimize the problem by loading some files ahead of time based on what the user can specify in its configuration files (see bug 487279 comment 14). This is the best we can do. Going back to consider only the current file would break the whole idea of using tern.
With all latest changes made to support js project, I will close that one for now. If users complain, we don't load enough on startup, we will check what files need to be handled and add the support for it.
What is the bottom line? It the tooling no longer sensitive to the order that the files are selected when I DO NOT have a .tern-project file? (if I have the file, then the tooling uses that exclusively)
