Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

feat!: add no-loader-import-specifier rule (#122) #133

Merged
merged 1 commit into from
Jan 20, 2020

Conversation

wincent
Copy link
Contributor

@wincent wincent commented Jan 20, 2020

This rule prevents us from providing useless import specifiers when importing SCSS via the loader:

import useless from './styles.scss';

These modules should only be imported for their side-effects; ie:

import './styles.scss';

As noted in the docs, we might want to lint for other kinds of non-JS resources in the future, but for now ".scss" is the only file type that we actually have configured to load via this mechanism.

Test plan: Copy the code over to a liferay-portal repo (eg. with cp -R ~/code/eslint-config-liferay/{portal.js,plugins} node_modules/eslint-config-liferay) and then introduce a fake error; see this in the lint run output (yarn checkFormat):

modules/apps/frontend-js/frontend-js-components-web/src/main/resources/META-INF/resources/treeview/Treeview.js
  24:1  error  SCSS resources should be imported only for side-effects  liferay-portal/no-loader-import-specifier

Closes: #122

This rule prevents us from providing useless import specifiers when
importing SCSS via the loader:

    import useless from './styles.scss';

These modules should only be imported for their side-effects; ie:

    import './styles.scss';

As noted in the docs, we might want to lint for other kinds of non-JS
resources in the future, but for now ".scss" is the only file type that
we actually have configured to load via this mechanism.

Test plan: Copy the code over to a liferay-portal repo (eg.
with `cp -R ~/code/eslint-config-liferay/{portal.js,plugins}
node_modules/eslint-config-liferay`) and then introduce a fake error;
see this in the lint run output (`yarn checkFormat`):

    modules/apps/frontend-js/frontend-js-components-web/src/main/resources/META-INF/resources/treeview/Treeview.js
      24:1  error  SCSS resources should be imported only for side-effects  liferay-portal/no-loader-import-specifier

Closes: #122
@jbalsas
Copy link
Contributor

jbalsas commented Jan 20, 2020

We could add .soy to the mix here... even if just for the sake of correctness and until we've gotten rid of them all...

@wincent
Copy link
Contributor Author

wincent commented Jan 20, 2020

We could add .soy to the mix here...

Ah, good idea!

@jbalsas
Copy link
Contributor

jbalsas commented Jan 20, 2020

Although now that I think of it, we might actually be using their return, so scratch that, I need coffee ☕️

We are loading some .js files just so they load .soy templates, but that'll be a whole different story

@wincent
Copy link
Contributor Author

wincent commented Jan 20, 2020

Common pattern:

import './ValidationRegister.soy.js';

Soy.register(Validation, templates);

and:

import './TextRegister.soy.js';

@wincent wincent merged commit 33304bf into master Jan 20, 2020
@wincent wincent deleted the wincent/no-loader-import-specifier branch January 20, 2020 09:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lint for inappropriate use of SCSS/CSS import return values
2 participants