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

New faster resolver supporting absolute and tilde paths, and aliases #850

Merged
merged 10 commits into from Feb 22, 2018

Conversation

Projects
None yet
@devongovett
Copy link
Member

devongovett commented Feb 19, 2018

This replaces the browser-resolve and resolve modules with our own custom resolver which is both faster and more extendible. It especially speeds up building from cache: on one project my build time went from 4.93s to 1.40s.

This performance increase is due to severely fewer fs.stat calls compared to the resolve implementation: for example, resolving one dependency I had in my project caused 258 fs.stat calls in resolve whereas it takes just 9 in the new resolver.

In addition to performance, there is also some new features:

  1. Absolute paths (e.g. /foo) resolved relative to the project root (even from within node_modules - is this a bad idea?). Closes #336, closes #290, closes #86.
  2. Tilde paths (e.g. ~/foo) resolved relative to the nearest package root in node_modules, or the project root if not in node_modules. Closes #336, closes #466.
  3. Aliases are supported using a package.alias field in package.json. It works identically to the browser field, but when found in the project root package.json it applies globally rather than to only the local module. The alias field also works within node_modules, but does not apply globally. Closes #25, closes #832.

As an example, you could declare the following in your project's package.json to alias react to preact globally:

{
  "alias": {
    "react": "preact-compat",
    "react-dom": "preact-compat"
  }
}

This can also be used to alias modules that don't exist in node_modules at all:

{
  "alias": {
    "fake-module": "./some/deep/path"
  }
}

Now you can require('fake-module') and it will point to the path you've specified.

@JounQin

This comment has been minimized.

Copy link

JounQin commented Feb 19, 2018

What about alias modules under specific directory? Or it can be alias one by one?

Like: https://webpack.js.org/configuration/resolve/#resolve-modules

@devongovett

This comment has been minimized.

Copy link
Member Author

devongovett commented Feb 19, 2018

@JounQin yeah, I don't think we are going to support something like that (similar to NODE_PATH environment variable). I think it's a bad idea to do this it makes it much harder to reason about where the code you read in a project is coming from. At least when you explicitly list the modules you want to alias in package.json there is one place to look for to see the mapping.

@Slapbox

This comment has been minimized.

Copy link

Slapbox commented Feb 19, 2018

Does this support mapping of named exports? For example webpack supports this syntax:

alias: ['nodeModuleName, exportName]

ipcRenderer: ['electron', 'ipcRenderer']

@devongovett

This comment has been minimized.

Copy link
Member Author

devongovett commented Feb 19, 2018

This is a mapping of files only. If you wanted to do something like that I guess you could do it by mapping to your own file which re-exports the thing you want.

{
  "alias": {
    "ipcRenderer": "./electron-ipc.js"
  }
}

in electron-ipc.js:

module.exports = require('electron').ipcRenderer;

devongovett added some commits Feb 19, 2018

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Feb 20, 2018

Codecov Report

Merging #850 into master will decrease coverage by 0.85%.
The diff coverage is 99.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #850      +/-   ##
==========================================
- Coverage   91.98%   91.12%   -0.86%     
==========================================
  Files          68       68              
  Lines        3404     3392      -12     
==========================================
- Hits         3131     3091      -40     
- Misses        273      301      +28
Impacted Files Coverage Δ
src/WorkerFarm.js 91.93% <100%> (+0.41%) ⬆️
src/Resolver.js 100% <100%> (ø) ⬆️
src/utils/syncPromise.js 100% <100%> (ø)
src/Parser.js 100% <100%> (ø) ⬆️
src/assets/StylusAsset.js 83.54% <66.66%> (-3.8%) ⬇️
src/utils/loadPlugins.js 64.28% <0%> (-35.72%) ⬇️
src/utils/installPackage.js 61.9% <0%> (-29.21%) ⬇️
src/utils/prettyError.js 63.63% <0%> (-22.73%) ⬇️
src/assets/CSSAsset.js 81.73% <0%> (-11.87%) ⬇️
src/visitors/fs.js 79.86% <0%> (-11.74%) ⬇️
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d9d14c...b23c03a. Read the comment docs.

@devongovett devongovett merged commit 32c38e5 into master Feb 22, 2018

3 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@devongovett devongovett deleted the resolver branch Feb 22, 2018

@davidnagli davidnagli referenced this pull request Feb 23, 2018

Merged

Automatically install missing dependencies, part 2 #805

3 of 3 tasks complete
@Cliff122

This comment has been minimized.

Copy link

Cliff122 commented Feb 24, 2018

I have a SCSS file containing @import "~bootstrap/scss/bootstrap"; which I import into my main app.js like this: import '../scss/styles.scss'. I still however get this error in the console when trying to compile.
C:\Users\david\Coding\bootstrap-test\src\scss\styles.scss:1:1: File to import not found or unreadable: ~bootstrap/scss/bootstrap.

What am I doing wrong?

@DeMoorJasper

This comment has been minimized.

Copy link
Member

DeMoorJasper commented Feb 24, 2018

@Cliff122 This hasn’t been released to npm. You could link to github. ‘~bootstrap...’ should probably also be ~/bootstrap...

Sent with GitHawk

@Cliff122

This comment has been minimized.

Copy link

Cliff122 commented Feb 24, 2018

Alright, I took your advice and added a / to the path as well as cloned the repo and built it locally on my machine. But even when I run the project using that version the error still appears. It looks like the error is thown by the node-sass module, if that is relevant.

@DeMoorJasper

This comment has been minimized.

Copy link
Member

DeMoorJasper commented Feb 24, 2018

@Cliff122 Aha! I remember a similar issue, SASS uses it's internal resolver unfortunately, as far as i'm aware, you can read up the whole discussion here: #316 it basically means it's out of parcel's control.

@Cliff122

This comment has been minimized.

Copy link

Cliff122 commented Feb 24, 2018

Ok, that makes sense. When importing directly in the javascript file however, like this: import '~/bootstrap/scss/bootstrap.scss', SASS shouldn't have anything to do with resolving the path, right? When I try that though I instead get the error C:\Users\david\Coding\bootstrap-test\src\scripts\app.js:1:7: Cannot resolve dependency '~/bootstrap/scss/bootstrap.scss'.

@DeMoorJasper

This comment has been minimized.

Copy link
Member

DeMoorJasper commented Feb 24, 2018

@Cliff122 hmmm not sure why u get an error, might be u are using it incorrectly, your import would resolve like this I think.
~/bootstrap/scss/bootstrap.scss => C:\Users\david\Coding\bootstrap-test\src\bootstrap/scss/bootstrap.scss
at least if the entrypoint is inside C:\Users\david\Coding\bootstrap-test\src\

@devongovett

This comment has been minimized.

Copy link
Member Author

devongovett commented Mar 1, 2018

@ellsclytn yes, it works as you describe.

@ckken this PR hasn't been released to npm yet. That should work in the next release.

@lifeiscontent

This comment has been minimized.

Copy link

lifeiscontent commented Mar 4, 2018

@devongovett I can't seem to get tilde paths to work... which version of parcel is this?

@davidnagli

This comment has been minimized.

Copy link
Member

davidnagli commented Mar 4, 2018

@lifeiscontent It hasn’t been released yet... Big release coming soon!

@derolf

This comment has been minimized.

Copy link

derolf commented Mar 5, 2018

Aliasing from HTML files using the tilde syntax doesn‘t work. Will this also come?

@derolf

This comment has been minimized.

Copy link

derolf commented Mar 5, 2018

I am working on the „externals“ issue. It would be very helpful if we could move the resolution step into the parse phase. This way aliasing works from ALL assets, i.e. you could write <script src=‘react/cjs/minified.js’> in HTML.

@devongovett devongovett added this to the v1.7.0 milestone Mar 9, 2018

@Brouilles

This comment has been minimized.

Copy link

Brouilles commented Mar 9, 2018

@davidnagli An approximation for the release of this update? So excited!

@Siyfion

This comment has been minimized.

Copy link

Siyfion commented Mar 11, 2018

Does this also allow imports for modules residing in a monorepo root node_modules folder?

i.e. Will ~/example search up the directory tree until it finds a node modules folder containing the example module, or will it stop when it gets to the project root folder?

@fathyb fathyb referenced this pull request Mar 17, 2018

Merged

Fix scoped modules #1013

1 of 1 task complete

@davidnagli davidnagli referenced this pull request Mar 22, 2018

Closed

Document Parcel v1.7.0 features #132

2 of 3 tasks complete

@sebald sebald referenced this pull request Mar 23, 2018

Open

Absolute module paths #5

@wesleyboar

This comment has been minimized.

Copy link

wesleyboar commented Apr 1, 2018

Has 1.7.0 has been released? I see 1.7.0 on master. I am very much looking forward to some documentation.


I am trying to use ~/ or / for protject-root-relative paths—and I am failing…

The nearest package.json is at /omg/absolute/path/scripts/package.json.

The import line is import * as IsLoggedIn from '[…]src/site/visual/is-logged-in.js';.

Absolute Paths

/omg/absolute/path/scripts/src/site/site.js:21:28: Cannot resolve dependency '/src/site/visual/is-logged-in.js' at '/src/site/visual/is-logged-in.js'

Tilde Paths

/omg/absolute/path/scripts/src/site/site.js:21:28: Cannot resolve dependency '~/src/site/visual/is-logged-in.js' at '/omg/absolute/path/scripts/src/site/~/src/site/visual/is-logged-in.js'


My workaround is to use babel-plugin-module-resolver, but my gut feels that the module resolution is the job of a bundler, not a transpiler.

@freeall

This comment has been minimized.

Copy link

freeall commented Apr 3, 2018

I have the same issue as @wesleyboar. I have been used to doing something like import Foo from ~/components/Foo. Even in node core I remember this having been a longer discussion at one point.

To me it has some great advantages:

  1. It allows us to quickly write requires/imports without thinking about how many ../ we have to put first.
  2. When moving code around you don't also have to look through requires/imports and change all that code, which can lead to a reason not to move code to the appropriate place.

Since you've already implemented reading alias from package.json it would feel natural to add it to that api as well as something like this:

{
  "alias": {
    "~": "./src/app/"
  }
}
@3stacks

This comment has been minimized.

Copy link

3stacks commented Apr 3, 2018

^ Maybe submit a new issue for this. Your request might get lost in a merged PR

@Stradivario

This comment has been minimized.

Copy link

Stradivario commented Jun 24, 2018

Any news about this ?

@goyney

This comment has been minimized.

Copy link

goyney commented Jun 26, 2018

It seems that folders aliased via the package.json file are not being transpiled by babel.

  "alias": {
    "clone-object": "../modules/clone-object",
    ....
   }

Instructing babel via a .babelrc file to transpile down to es5 doesn't apply to these aliased folders. How can we make that work?

DeMoorJasper added a commit to parcel-bundler/website that referenced this pull request Aug 25, 2018

document module resolution strategies (#229)
This is my initial attempt at resolving #184, #132 and documenting parcel-bundler/parcel#850

I have a few questions which I believe should be answered and better described before merging:
- I would like to answer this question in the docs parcel-bundler/parcel#850 (comment)
- clarify terminology of:
	- project root: top-level package.json?
	- package root: package.json in node_module module? where does this start to resolve from?
- these comments https://github.com/parcel-bundler/parcel/pull/850/files#diff-f90b0caefa8028ff5ec8f2f2c2e6e39dR8 seem to suggest I may be missing some important information that should be documented

Closes #184
@domenkozar

This comment has been minimized.

Copy link

domenkozar commented Oct 2, 2018

For future me, this is now documented in https://parceljs.org/module_resolution.html

michaelwooley added a commit to michaelwooley/react-parcel-sass-electron-boilerplate that referenced this pull request Oct 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment