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

Feat: Create js alias for root/app/js #403

Closed
wants to merge 7 commits into from

Conversation

vibalre
Copy link
Member

@vibalre vibalre commented Dec 2, 2022

Changes

Closes #77

How to test

  • Build and install Rhino 1.2.1.9000 using this branch.
  • Create a new Rhino project.
  • Create JS modules in app/js or subdirectories in app/js.
  • Import modules relative to app/js using js alias. E.g. if module is in app/js, use js/name_of_module; if module is in app/js/dir, use js/dir/name_of_module.
  • rhino::lint_js() and rhino::build_js() should work.

Issue #77

Found the best explanation here:
https://webpack.js.org/configuration/resolve/#resolvealias

With this setting, all files and folders in js can now
be referenced with the js alias,
e.g. a module app/js/module.js should be imported in
index.js as js/module.js.

app/js/foo/hello.js should be imported in index.js as
js/foo/hello.js.
path.resolve is necessary to make the alias work.
Lint should now use the same alias/path from webpack,
i.e. js=root/app/js.

parserOptions : {requireConfigFile: false}
This was set to false because reading modules from the
aliased path get an error about no babel config found.

import/no-extranesous-dependencies: warn
I had a path with a space in the folders, and that
triggerd an error that I should install the folder
via npm i folder-name.
This allows the use of js/ when refering to js scripts
in app/js.
This is possible because of
eslint-import-resolver-webpack.
@kamilzyla
Copy link
Collaborator

kamilzyla commented Dec 5, 2022

Unfortunately I found some issues when testing this PR. Steps to reproduce:

  1. Initialize a fresh Rhino application.
  2. Add import 'js/hello'; to app/js/index.js.
  3. Create an empty app/js/hello.js file.
  4. Run rhino::lint_js().

This results in the following error (made slightly clearer after upgrading to ESLint 8.29.0):

> lint-js
> eslint --config .eslintrc.json ../app/js


Oops! Something went wrong! :(

ESLint: 8.29.0

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received null
Occurred while linting /home/kamil/git/box/rhino/app/js/index.js:1
Rule: "import/no-extraneous-dependencies"
    at new NodeError (node:internal/errors:387:5)
    at validateString (node:internal/validators:162:11)
    at dirname (node:path:1276:5)
    at getFilePackagePath (/home/kamil/git/box/rhino/.rhino/node_modules/eslint-plugin-import/lib/core/packagePath.js:15:641)
    at getContextPackagePath (/home/kamil/git/box/rhino/.rhino/node_modules/eslint-plugin-import/lib/core/packagePath.js:15:420)
    at isExternalPath (/home/kamil/git/box/rhino/.rhino/node_modules/eslint-plugin-import/lib/core/importType.js:127:1565)
    at typeTest (/home/kamil/git/box/rhino/.rhino/node_modules/eslint-plugin-import/lib/core/importType.js:127:2646)
    at resolveImportType (/home/kamil/git/box/rhino/.rhino/node_modules/eslint-plugin-import/lib/core/importType.js:127:2873)
    at reportIfMissing (/home/kamil/git/box/rhino/.rhino/node_modules/eslint-plugin-import/lib/rules/no-extraneous-dependencies.js:170:35)
    at commonjs (/home/kamil/git/box/rhino/.rhino/node_modules/eslint-plugin-import/lib/rules/no-extraneous-dependencies.js:267

It seems to be an issue with eslint-import-resolver-webpack, still not resolved after almost two years.

Possible solutions:

  1. Drop Webpack.
  2. Downgrade Webpack to v4 (undesirable).
  3. Disable the import/no-extraneous-dependencies rule (undesirable).

It is unlikely that we'll do (1) soon, so I'm archiving this PR as archive/2022-12-02/root-relative-js-imports and marking the issue as blocked.

@kamilzyla kamilzyla closed this Dec 5, 2022
@kamilzyla kamilzyla deleted the feat/allow-root-relative-js-imports branch December 5, 2022 13:56
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.

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