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

document module resolution strategies #229

Merged
merged 10 commits into from
Aug 25, 2018
Merged

document module resolution strategies #229

merged 10 commits into from
Aug 25, 2018

Conversation

jthegedus
Copy link

@jthegedus jthegedus commented Aug 23, 2018

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:

Closes #184

jthegedus added 3 commits August 23, 2018 10:59
add section on monorepo resolution
better explain JS named export aliasing
}
```

Avoid using `~` in your aliases as it is already used by parcel as mentioned earlier.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users should probably avoid using any special characters all together.

@ will try to resolve npm orgs
~ will try to resolve tilde paths

and more characters might be added in the future either from npm, parcel or some third party extension/plugin


## Absolute Paths

`/foo` will resolve `foo` relative to the **project root** (top-level `package.json`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't correct, project-root isn't related to package.json as package.json is an optional file and therefore makes it impossible for us to use it as a project root.

Project root is the directory of the entrypoint or the shared root in case of multiple entrypoints, you can have a look at how it works here: https://github.com/parcel-bundler/parcel/blob/master/src/utils/getRootDir.js


### Monorepo Resolution

TODO: answer this - https://github.com/parcel-bundler/parcel/pull/850#issuecomment-372105317
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I've tried tilde paths don't work inside aliased packages and monorepos, I might be wrong though.

My advice would be to avoid tilde paths altogether and use relative paths as much as possible and preferably as explicit as possible (use extensions and such). And if you really need a root path, use /

James Hegedus added 4 commits August 25, 2018 13:56
* fill out monorepo section
* better advice around special characters as aliases
* fix error about project-root terminology
* clarify feedback comment applies to monorepo users
* fix noun capitalisation
@jthegedus
Copy link
Author

@DeMoorJasper Thanks for the feedback, I've made updates which I think provide sufficient fixes 😃

James Hegedus added 2 commits August 25, 2018 14:28
* fix json codeblock so github diff isn't angry
* properly  fix json codeblock so github diff isn't angry
Advised usage:

- use relative paths.
- be as explicit as possible (use file extensions).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't scoped at monorepo's this should be advised across all code as not using extensions causes Parcel to guess the extension possibly linking the wrong file. (if 2+ files would have the same name but diff extension)

Everything else looks good :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah righto. Updated 😃

* move recommendation of file ext specificity to alias section
@DeMoorJasper DeMoorJasper merged commit ea55206 into parcel-bundler:master Aug 25, 2018
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.

docs: add section on module resolution
2 participants