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

New resolver implementation in Rust #8807

Merged
merged 82 commits into from
Feb 19, 2023
Merged

New resolver implementation in Rust #8807

merged 82 commits into from
Feb 19, 2023

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Feb 2, 2023

This is a new implementation of Parcel's resolver. It includes a few new features to improve compatibility with other tools:

It also replaces 3 separate node resolver implementations with a single one. Previously, we had separate implementations for the default resolver plugin vs in the NodePackageManager. Now they all use a single library, written in Rust, which provides lots of feature flags to enable turning on or off different resolver features. This allows us to match Node's behavior when needed, and optionally turn on additional features to match TypeScript, webpack, previous Parcel versions, etc. It will also enable future features like ESM plugins, and prepare us for the future when more of Parcel core is written in Rust.

Since it is written in Rust, most file system operations can happen natively if you are using a NodeFS (and Yarn PnP is not enabled). Otherwise, we pass in JS callbacks that are called to read files. The API in the @parcel/node-resolver-core package is now a wrapper around the Rust implementation providing the same API as before (as long as you weren't using internal methods). Yarn PnP, diagnostics, and some other things like checking dependency versions and resolving builtin polyfills are still done in JS since they are rare and not performance critical.

For package.json exports, we try to match the conditions that webpack applies based on Parcel's environment, though not all of them are supported. Currently, there is no way to specify custom conditions - not sure how we would want to do that.

There are a few questions about how some of these resolution features should work together. For example, should aliases apply to package.json exports or tsconfig paths? Would they go before or after? etc. See some comments in the code for more.

To do

  • Clean up code
  • Performance testing
  • Run a lot more test to ensure compatibility with other tools

@devongovett
Copy link
Member Author

A follow up I'd like to do to improve performance is to create an instance of NodeResolver once per build rather than once per request. This way the cache can actually be reused. This will require a new method be introduced for Resolver plugins that runs on build start in order for the instance to be created. Not sure what to call that. Is it the same as loadConfig? If we added that, when would it run?


// https://url.spec.whatwg.org/#query-state
fn parse_query<'a>(input: &'a str) -> (Option<&'a str>, &'a str) {
if !input.is_empty() && input.as_bytes()[0] == b'?' {
Copy link
Member

@mischnic mischnic Feb 18, 2023

Choose a reason for hiding this comment

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

as_bytes()[0] == b'?' is probably incorrect with multibyte characters? It can never be a starting byte in a multibyte utf8 character...

Comment on lines -21 to -27
// Extensions are always required in URL dependencies.
extensions:
dependency.specifierType === 'commonjs' ||
dependency.specifierType === 'esm'
? ['ts', 'tsx', 'mjs', 'js', 'jsx', 'cjs', 'json']
: [],
mainFields: ['source', 'browser', 'module', 'main'],
Copy link
Member

Choose a reason for hiding this comment

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

This could be a breaking change for some plugins. Should we try to keep supporting this? Or was the plan to do a major bump on node-resolver-core anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it's a bit hard to support custom main fields with the way package.jsons are parsed now (rust discards unknown fields). Extensions could be done but idk if there are any use cases other than replicating the default logic from here. I think the safest thing would be to bump a major anyway because none of the methods other than resolve exist anymore, and someone might have been using them.

Comment on lines 182 to 183
// TODO: invalidations?
return {isExcluded: true};
Copy link
Member

Choose a reason for hiding this comment

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

Changing aliases/exports/... could probably influence whether it's external or not?

Comment on lines 46 to 47
/// The "exports" field in package.json.
const EXPORTS = 1 << 5;
Copy link
Member

Choose a reason for hiding this comment

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

That's for both exports and imports, right?

}

fn resolve_node_module(&self, module: &str, subpath: &str) -> Result<Resolution, ResolverError> {
// TODO: check if module == self
Copy link
Member

Choose a reason for hiding this comment

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

Still relevant?

Copy link
Member Author

Choose a reason for hiding this comment

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

The node docs have a section on this, and it's even got a special PACKAGE_SELF_RESOLVE function in the spec. But I don't really understand why it's called out specifically, because self references work even without this special handling just by searching the node_modules folders as with any other package. I added some tests for it but 🤷

@mischnic
Copy link
Member

Should it also read from jsconfig.json?

let tsconfig = await config.getConfigFrom<TSConfig>(
options.projectRoot + '/index',
['tsconfig.json', 'jsconfig.json'],
);

@devongovett
Copy link
Member Author

Should it also read from jsconfig.json?

Yeah maybe but it would add yet another file that needs to be checked for in every directory. Also other tools like esbuild don't check for it either. I want to start with supporting tsconfig.json and see if we get feedback about needing jsconfig.json too.

@devongovett devongovett merged commit 307268c into v2 Feb 19, 2023
@devongovett devongovett deleted the rust-resolver branch February 19, 2023 23:52
@fregante
Copy link
Contributor

TypeScript and exports? 😱 🎉

@jboler
Copy link

jboler commented Feb 23, 2023

Will this new resolver support import path aliases starting with @ ? e.g.

  // package.json
  "alias": {
    "@/**": "/src/$1"
  },

  // component.tsx
  import { thing } from '@/components/...'

@fregante
Copy link
Contributor

fregante commented Feb 23, 2023

Doesn't it already? I think you just need "@/**/*": "./src/$1/$2"

@jboler
Copy link

jboler commented Feb 23, 2023

@fregante You're absolutely right - thanks for that!

@nastynaz
Copy link

Does this actually work? I've been stuck for 4 hours trying to get it working. I'm on parcel 2.9.0. Fresh repo with the following tsconfig:

{
  "compilerOptions": {
    "target": "es2022",
    "paths": {
      "shared/*": ["./src/shared/*"]
    },
    "include": ["src/**/*"]
  }
}

After running parcel build and then running the code with node, I get a "cannot find package 'shared'" error. The code doesn't exist but the imports are still trying to import a nonexistant 'shared' folder.

I've also tried adding various aliases to my package.json to no avail. If anyone could help me I'd greatly appreciate it!

@fregante
Copy link
Contributor

fregante commented Jun 29, 2023

I think it’s opt-in at the moment. You need to customize it in your parcelrc file:

https://parceljs.org/blog/v2-9-0/#new-resolver

@nastynaz
Copy link

nastynaz commented Jul 1, 2023

@fregante Unfortunately that doesn't work.

@devongovett Since Parcel uses swc under the hood, is it possible to just change the .swcrc file/config it uses?

Because of this issue I went back to using swc. It's pretty easy to set up there, you just copy your tsconfig.json paths statement directly into the .swcrc:

{
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "decorators": true
    },
    "target": "esnext",
    "paths": {
      "opensea/*": ["./src/shared/opensea/*"]
    }
  },
  "module": {
    "type": "commonjs"
  },
  "sourceMaps": true
}

marcins pushed a commit to marcins/parcel that referenced this pull request Jul 14, 2023
* upstream/v2:
  Missing edge for multiple targets (parcel-bundler#8854)
  Split large runtime manifest into separate bundles (parcel-bundler#8837)
  Improvements to new resolver (parcel-bundler#8844)
  Fix published files for resolver
  New resolver implementation in Rust (parcel-bundler#8807)
  Update yarn.lock (parcel-bundler#8843)
  Bump napi-rs to latest (parcel-bundler#8838)
lettertwo added a commit that referenced this pull request Nov 6, 2023
* upstream/v2: (128 commits)
  [webextension] Add support for `chrome_style` (#8867)
  Switch to SWC minifier by default (#8860)
  Use BitSet for bundler intersections (#8862)
  best key logic truncating package names (#8865)
  Add support for loadConfig to resolver plugins (#8847)
  Missing edge for multiple targets (#8854)
  Split large runtime manifest into separate bundles (#8837)
  Improvements to new resolver (#8844)
  Fix published files for resolver
  New resolver implementation in Rust (#8807)
  Update yarn.lock (#8843)
  Bump napi-rs to latest (#8838)
  Support .proxyrc.cjs  (#8833)
  Sort global deps before injecting imports (#8818)
  Sort CSS module exports (#8817)
  fix: add extra information to unique bundles (#8784)
  Don't blow up HMR when <link />s don't have hrefs (#8800)
  v2.8.3
  Changelog for v2.8.3
  Address bug by updating an asset reference and merge conditions (#8762)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants