| Summary: | [regression] Can no longer get node content assist | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Michael Rennie <Michael_Rennie> |
| Component: | JS Tools | Assignee: | Michael Rennie <Michael_Rennie> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P2 | CC: | Carolyn_MacLeod, curtis.windatt.public, remy.suen, Silenio_Quarti, steve_northover |
| Version: | 13.0 | ||
| Target Milestone: | 14.0 | ||
| Hardware: | PC | ||
| OS: | Mac OS X | ||
| Whiteboard: | |||
Can't get assist to work at all in the CodeEdit demo: http://libingw.github.io/OrionCodeEdit/ P1 ... no code assist! (In reply to Steve Northover from comment #2) > P1 ... no code assist! Fixed the cross-file assist not working: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=2ae6f76073059aa44917be9a04f4c4b0e17bf6f1 this fixes points 2, 5, 6, and 7. (3) activate content assist at the end of 'mod'. Notice it is talking about commonjs (BAD) Node plugin, starts CommonJS plugin which has the definitions for modules/require/exports inside the module scope that modules.js creates. However, it also includes a "modules" definition outside of its scope (global scope). So modules always shows up as a proposal when the plugin is running. The issue must be some difference between the scopes modules.js is creating and what the commonjs plugin expects. They are expecting the entire file's contents to be in a 'module' type. When this is true you would see proposals (with node.js doc) for require, exports, parent, children, etc. From the 'module' doc: A reference to the current module. In particular module.exports is the same as the exports object. module isn't actually a global but rather local to each module. --------------------------------- (4) remove 'mod' and enter 'requ' and activate content assist - you do not get the proposal for the 'require' function call (BAD) In modules.js we only create a module scope if you are using node dependencies. Running the plugin is not enough, you need env.node set, which only happens when you already have a require() statement in the file. If you add require(1, 2); then later type req and open content assist, it will offer 'require(id)' as a completion as you would expect. We experimented with changing the default behaviour previously, but it prevents requireJS plugin from running correctly when node plugin is running. Reducing severity now that the content assist not working problem is fixed. Also, 3 and 4 are not regressions. As we are much smarter about which plugins get started, I would be ok with assuming node environment in (4). We would have to check if the requireJS plugin is running (situation in Orion client today) and whether no module plugin is running (minesweeper HTML demo). (In reply to Curtis Windatt from comment #6) > As we are much smarter about which plugins get started, I would be ok with > assuming node environment in (4). We would have to check if the requireJS > plugin is running (situation in Orion client today) and whether no module > plugin is running (minesweeper HTML demo). I implemented this, but it is not complete without the fix for the CommonJS definition (3). The require() proposal only shows up if you have used a require() previously because Tern is guessing. Also encountered weird behaviour with the plugins all being started, see Bug 512595. (In reply to Curtis Windatt from comment #7) > (In reply to Curtis Windatt from comment #6) > > As we are much smarter about which plugins get started, I would be ok with > > assuming node environment in (4). We would have to check if the requireJS > > plugin is running (situation in Orion client today) and whether no module > > plugin is running (minesweeper HTML demo). > > I implemented this, but it is not complete without the fix for the CommonJS > definition (3). The require() proposal only shows up if you have used a > require() previously because Tern is guessing. > > Also encountered weird behaviour with the plugins all being started, see Bug > 512595. I ran an experiment where I removed all of our AST env and scope checks from the requirejs, node, commonjs and modules plugins, and everything worked as expected. Rather than think about heaping on more customizations, we should investigate using the plugins as-is from Tern (or as close to it as we can get). (In reply to Michael Rennie from comment #8) > I ran an experiment where I removed all of our AST env and scope checks from > the requirejs, node, commonjs and modules plugins, and everything worked as > expected. Problem cases previously: a) If node plugin is running, can you use HTML script imports and global properties. b) If node and requirejs plugins are both running what do you get in a file that has a define statement, a file that has a require statement and a file with neither. All my investigations indicate that commonjs plugin is working as designed. The plugin just isn't as helpful as we want for completions and the origin. The plugin propagates its properties into the 'module' scope which ends up a) Removing any doc/url and b) Sets the origin to the file. The only property definition added to the server is 'modules', which has the correct doc and 'CommonJS' origin. I'm closing this as FIXED for 14.0 and will open a new bug to discuss altering the plugin defs in 15.0. Bug 513025 - [Tern] Investigate improving assist proposals for CommonJS Bug 513026 - [Tern] Investigate removing our use of ast.environment, at least in modules.js Trying to understand exactly which cases work and which ones do not. Apologies if this has already been spelled out in detail. (In reply to Steve Northover from comment #11) > Trying to understand exactly which cases work and which ones do not. > Apologies if this has already been spelled out in detail. See comment #0, all cases were fixed except for 3 and 4. > 3. activate content assist at the end of 'mod'. Notice it is talking about > commonjs (BAD) Bug 513025 - Improve CommonJS definition use > 4. remove 'mod' and enter 'requ' and activate content assist - you do not > get the proposal for the 'require' function call (BAD) > lib. This works if you have a require() already in the file. Investigating removing that as a requirement is Bug 513026. |
Steps: 1. create a simple project with a .tern-project file with the following contents: { "plugins": { "node": {} }, "ecmaVersion": 6, "libs": [ "ecma5", "ecma6", "browser" ], "loadEagerly": [] } 2. create a file a.js with the following contents: mod 3. activate content assist at the end of 'mod'. Notice it is talking about commonjs (BAD) 4. remove 'mod' and enter 'requ' and activate content assist - you do not get the proposal for the 'require' function call (BAD) 5. Create a second file b.js with the following contents: module.exports = { myFunc: function myFunc() { } }; 6. back in a.js add the following: /* eslint-env node*/ var lib = require('./b'); lib. 7. try assist after 'lib.'. You get no proposals (BAD).