Configure root-relative imports for JS (avoid `import '../../..'`)

[kamilzyla]

Background

A root-relative path is a path interpreted relative to the project root. For example, in app/view/table.R you can have box::use(app/logic/constants) and it will be interpreted as app/logic/constants.R and NOT e.g. app/view/app/logic/constants.R.

Problem

Our current setup doesn't allow root-relative import paths in JS. Say we have files app/js/foo/hello.js and app/js/bar/bye.js. You need to use .. to import bye.js from hello.js:

import '../bar/bye.js';

Using .. in import statements makes the code less readable and maintainable. It would be preferable to do something like this instead:

import 'app/js/bar/bye.js'; // Doesn't work as of now.

Notes

We can probably achieve this via some Webpack config. Consider what would be the best prefix to use for root-relative paths: app/js/ seems the most natural but somewhat verbose; it could be e.g. js/ or @js/ (interpreted relative to app/).

[vibalre]

Is the goal here to ensure that all JS files should be in app/js, such that import { jsFunction } from '../../../jsScript' will not work and fail with rhino::build_js(), or at least fail rhino::lint_js()?

[kamilzyla]

@vibalre I updated the issue description; I hope it is clear now :slightly_smiling_face:

[vibalre]

@kamilzyla so far I have only figured out to make it work with rhino::build_js().

We can configure webpack to resolve an alias with path.resolve. I have tested, and it works in my machine.

My problem now is how I can do the same with rhino::lint_js(). I found out that we can specify the same alias in dot.eslintrc.json, but this will require the eslint-import-resolver-alias, can we include this in yarn.lock?

I see that we have eslint-import-resolver-node, but I have been trying with paths and moduleDirectory, but I can't make it work with rhino::lint_js(). I always get the error that the path could not be resolved.

[kamilzyla]

@vibalre Ideally with some configuration we'd get ESLint to to read the alias from the Webpack config so that we wouldn't need to repeat the alias definition (DRY). I think I had such a setup in one project - please research if we could do it in Rhino.

In general, the fewer dependencies the better, but if adding another package helps us- let's do it. Perhaps we should remove eslint-import-resolver-node then?

[vibalre]

@kamilzyla I agree about keeping it DRY as much as possible.

eslint-import-resolver-node is not in package.json, but I saw it in yarn.lock, so maybe it is a minor dependency, but installed by default.

Using Webpack Alias

I agree that we should be able to use the alias in webpack config. I learned that we also have eslint-import-resolver-webpack that allows us to use the alias from webpack in .eslintrc.json.

"settings": {
    "import/resolver": {
      "webpack": {
        "config": "./webpack.config.babel.js"
      }
    }
  }

However, I encountered two major errors: no babel config file detected and import/no-extraneous-dependencies due to a space in the path.

No Babel Config File

Error while parsing path/to/js
Line undefined, column undefined: No Babel config file detected for path/to/js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files.
`parseForESLint` from parser `proj-root\.rhino\node\node_modules\@babel\eslint-parser\lib\index.cjs` is invalid and will just be ignored

From the error above, I can confirm that eslint is using the correct alias and path to the js file (other modules used in index.js).

My solution for this was to follow the error's suggestion and set requireConfigFile: false.

// add to .eslintrc.json
"parserOptions": {
    "requireConfigFile": false
  }

I am assuming the parser will then use the default babel config file. Correct me if I am wrong.

Space in the Path

My user folder unfortunately has a space, e.g. instead of UserName it is User Name.

When I run rhino::lint_js() with the webpack alias included and require config file disabled in parser, I get an error like this:

C:\Users\User Name\project-root\.rhino\node\root\app\js\index.js
  3:1  warning  'user-name' should be listed in the project's dependencies. Run 'npm i -S user-name' to add it  import/no-extraneous-dependencies

I spent some time trying to figure out how to fix this error, but I could not find a good solution. I guess it is not good practice to have a space in the user folder (or any file path), but I think spaces in paths is not uncommon, so this might become a recurring issue. My only workaround is to turn import/no-extraneous-dependencies into a warning rather than an error.

// add this rule to .eslintrc.json
"import/no-extraneous-dependencies": [
      "warn",
      {
        "devDependencies": true,
        "optionalDependencies": true,
        "peerDependencies": true
      }
    ]

I am suggesting this as a form of desperation. 😅

A possible alternative would be to use eslint-import-resolver-alias then write the same alias in .eslintrc.json (WET).

I converted this into a StackOverflow question.

[vibalre]

Current changes to rhino for this issue:

• webpack.config.babel.js with alias
• .eslintrc.json using webpack alias

[kamilzyla]

Thanks for your research @vibalre! I want to take a deeper look tomorrow, but here are a couple of notes for now:

1. The package.json lists direct dependencies: packages which are explicitly imported and used in the project. The yarn.lock file lists all direct as well as transitive dependencies (the dependencies of dependencies). If you want to import some package, it should be listed in the package.json.
2. The "no Babel config file" is probably due to our root link. It seems that somewhere along the way ./.node/root/app/js gets translated to ./app/js and thus ./.node/webpack.config.babel.js is no longer detected as it doesn't seem to be in any ancestor directory of ./app/js.
3. It would be fine to use requireConfigFile: false if we could confirm that the "root" Webpack config is used in that case (./.node/webpack.config.babel.js). It is not acceptable however if that means ESLint "will not parse any experimental syntax" (see "Additional paraser configuration" here) in files imported with a root-relative path: the linter behavior must be consistent regardless of how a file is imported.
4. The "should be listed in the project's dependencies" error is unlikely to be due to a space in your username - it also appears on my system (and my paths have no spaces).
5. We cannot turn (4) into a warning: if users can (should) use root-relative imports, they mustn't lead to warnings. We could disable that rule completely, but the implications would need to be carefully considered and it wouldn't be ideal.
6. How would a solution with eslint-import-resolver-alias look like? DRY is not an unbreakable rule, rather a principle worth following.

[vibalre]

eslint-import-resolver-alias

I added this dependency in package.json and updated everything with yarn.

It works, but I get the same error with eslint-import-resolver-webpack, so I think we should just stick with the current dependencies.

Babel Config File

I can't confirm that ./.node/webpack.config.babel.js is still being used when requireConfigFile: false. I did some tests and made simple lint errors in all the js files, and it seems that even with requireConfigFile: false linting is able to catch unused functions and missing semicolons. I do not understand what "experimental syntax" is, so I can't confirm that ESLint "will not parse any experimental syntax".

I tried looking for some answers in @babel/eslint-parser and Babel config options. The best chance I had was using

babelOptions: {
      configFile: "path/to/config.js",
    }

But I can't specify a path that works. I don't know what root Babel should take. I assume it should be .rhino/node since that contains package.json, but I cannot make it work. I tried different path configurations for path/to/webpack.config.babel.js, but I always get this error:

0:0  error  Parsing error: Cannot find module 'path/to/webpack.config.babel.js'

import/no-extraneous-dependencies

If you are getting the same error without spaces, then I am not sure what is causing this. This actually prevents imports for modules not declared in package.json. Does this mean, eslint is trying to check the imports within the project if they are not included in package.json? In that case, this will always throw an error when we try to import functions from our modules in app/js. For reference, in my tests, I use three import calls, and the import/no-extraneous-dependencies error does occur three times.

[kamilzyla]

As an experiment I tried the following:

1. Initialize a Rhino application and run rhino::lint_js() to get the .rhino/node directory.
2. Define alias in webpack.config.babel.js:

     resolve: {
       alias: {
         js: resolve(__dirname, 'root', 'app', 'js'),
       },
     },
   

3. Enable eslint-import-resolver-webpack in .eslintrc.json:

     "settings": {
       "import/resolver": "webpack"
     },
   

4. Attempt to use a root-relative import in index.js, e.g. import { hello } from 'js/hello'. Now rhino::lint_js() should fail with the errors as mentioned in previous comments.
5. Replace the .rhino/node/root link with a directory and move the whole project into it. Try running yarn lint-js in .rhino/node - it will now work fine.

Now some important things to notice: before step (5) we get "No Babel config file detected" and "import/no-extraneous-dependencies". It seems that the Babel issue is crucial though: same errors appear if we try to import a non-existent object, e.g. import { noSuchThing } from 'js/hello'. After step (5) ESLint correctly detects that we are importing a non-existent object.

Thoughts

All our problems here seem to stem from the fact that we are using the .rhino/node/root soft link. As we have other issues around it (e.g. #276 and problems on Windows in general) it is probably best to put this task on a shelf until we get rid of the root link.

[kamilzyla]

Marking as blocked after finding another obstacle.