Skip to content

Conversation

LinusBorg
Copy link
Contributor

@LinusBorg LinusBorg commented Feb 14, 2017

  • remove /src from resolve:module
  • change aliases to prevent mixup with npm modules.
  • update alias usage in source files.
  • update alias usage in test files.
  • test it.

@LinusBorg LinusBorg changed the title WIP: Cleanup bad aliases and resolve config WIP: Cleanup bad aliases and resolve config (fix #513) Feb 14, 2017
kazupon
kazupon previously approved these changes Feb 15, 2017
@squirmy
Copy link
Contributor

squirmy commented Feb 15, 2017

@LinusBorg

using the @ in the alias makes the import statements look they are importing a scoped module.
https://docs.npmjs.com/misc/scope

i picked this up because I tried it with the airbnb linted which complained they were an extraneous dependency.

perhaps it would be better to use a different character :)

@LinusBorg LinusBorg dismissed kazupon’s stale review February 15, 2017 14:39

dismissing because of squirmy's valid concern

@LinusBorg
Copy link
Contributor Author

LinusBorg commented Feb 15, 2017

@squirmy Oh, that's a valid concern of course. Suggestions for a better character?

Will have to try out $ or #

@simplesmiler
Copy link

simplesmiler commented Feb 15, 2017

How about '@': resolve('src')? It will be '@/src' in the code, which is just one symbol more then '@src', and is distinguishable from the scoped modules.

@limichange
Copy link
Contributor

How about ~ ? Like nuxtjs.

@LinusBorg
Copy link
Contributor Author

Doesnt that conflict with asset paths in css-loader?

@limichange
Copy link
Contributor

All thing is fine.
like this.

  import api from '~api'
  import avatar from '~components/avatar'

@LinusBorg
Copy link
Contributor Author

I mean, in cssl loader, ~ is a special charater to make css-loader aware that the asset path is a node_module path:

from the README:

To be compatible with existing css files (if not in CSS Module mode):

* `url(image.png) => require("./image.png")`
* `url(~module/image.png) => require("module/image.png")`

@LinusBorg
Copy link
Contributor Author

I think @simplesmiler 's suggestion is probably the best.

@squirmy
Copy link
Contributor

squirmy commented Feb 16, 2017

Here's some prior art:
https://github.com/entwicklerstube/babel-plugin-root-import

@LinusBorg
Copy link
Contributor Author

LinusBorg commented Feb 16, 2017

@squirmy Thanks! that would have thje added benefit of eslint support. Or would the eslint plugin also work without the babel one?

Will have a testrun with it!

@squirmy
Copy link
Contributor

squirmy commented Feb 16, 2017

@LinusBorg I think eslint is already happy with the current webpack alias solution. I believe this part of the config is making it work:

  'settings': {
    'import/resolver': {
      'webpack': {
        'config': 'build/webpack.base.conf.js'
      }
    }

Was more hinting at the fact that others have done the same concept as above and used @/ as the 'alias'.

I think @simplesmiler's suggestion would work well. It requires one alias, and things would be referenced like:

@/file-at-root
@/components/file
@/assets/file

Any other aliases for further down the tree can be added by the user. :-)

@LinusBorg
Copy link
Contributor Author

Ok then, I think that's good enough for me, will use @ -> /src

@LinusBorg LinusBorg self-assigned this Feb 22, 2017
@LinusBorg
Copy link
Contributor Author

Sorry that this is still unfinished, had a busy week. Hope to finish this in a quiet minute in the next days.

- only one alias. This is secure against mismatching @nAmed as well as regular npm packages
because '@/' looks too weird imho
@LinusBorg
Copy link
Contributor Author

LinusBorg commented Feb 24, 2017

Changed to @, but not tested locally yet.

@LinusBorg
Copy link
Contributor Author

Finally came around to testing this.

@LinusBorg LinusBorg merged commit 801bc02 into master Feb 25, 2017
@LinusBorg LinusBorg deleted the clean-up-bad-aliases branch February 25, 2017 10:54
@dfdgsdfg
Copy link

dfdgsdfg commented Mar 11, 2017

Seems ~assets alias not working with the latest template. Is this realate with this?

Also @/assets does not work.

<-- NOTE: Not work -->
<img src="~assets/img.png">
<img src="@/assets/img.png">

asset resolving rules doc

@LinusBorg
Copy link
Contributor Author

~@/assets/img.png

@LinusBorg
Copy link
Contributor Author

We should probably adjust the docs a little to reflect that, even though the included aliases where never thoroughly documented in the first place.

@dfdgsdfg
Copy link

dfdgsdfg commented Mar 11, 2017

@LinusBorg Thanks. 👍
~@/assets/img.png just works fine.

@AlexandreBonaventure
Copy link

@LinusBorg
Checkout: https://www.npmjs.com/docs/orgs/?utm_campaign=2017-03-22%20free%20Orgs&utm_medium=email&utm_source=Eloqua
From now on, npm allows free orgs, we should see more and more the use scope (eg. @org/packgage)
It will then enter in conflict with the actual alias @

@LinusBorg
Copy link
Contributor Author

No, it won't. The alias woeks on paths like @/components, not @components.

@AlexandreBonaventure
Copy link

OK, but maybe for the sake of readability (could be misleading for beginners), we should find another symbol. Just saying... Could provide a PR if accepted

@LinusBorg
Copy link
Contributor Author

We're open to suggestions. As you can see in the previous comments on this PR, we discussed a few.

@AlexandreBonaventure
Copy link

My bad, I noticed the subject was brought to the table before. But still I found the @ a bit misleading. Why not & or # as you suggested in the first place ?

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.

7 participants