rturnq / solid-router

A router for solid-js
MIT License
42 stars 1 forks source link

Package.json `source` point to a non existing folder #10

Closed amoutonbrady closed 4 years ago

amoutonbrady commented 4 years ago

Hey,

I'm using your package (which is quite brilliant I might add), with parcel 2. The problem is that parcel base its resolving on attributes found in package.json. Unfortunately, your package source point to a non packaged directory src which causes the following error:

/node_modules/@rturnq/solid-router/package.json:11:13
  10 |   },
> 11 |   "source": "src",
>    |             ^^^^^ './src' does not existundefined'
  12 |   "main": "dist/index.js",
  13 |   "module": "dist/index.esm.js",

Would you mind either removing that source key or actually packaging the src directory?

Thank you again for your library!

rturnq commented 4 years ago

Thanks for the kind words.

I removed the property and published a 0.1.5 version (0.1.4 was botched when I published it to npm) so you don't have to use the beta. Hopefully that solves the issue with Parcel.

I am interested though in how you encountered this issue. I have a couple projects which are using Parcel 2 and this router and hadn't seen this. I don't really see why Parcel would be looking at this property on a non-top-level package.json.

amoutonbrady commented 4 years ago

Thank you very much for the quick action! It totally fixed the issue for me.

I'm also curious as to how do you handle your parcel2 configuration. It's out of the scope of this issue but I have to ask; have you managed to build a project using solid with parcel 2 and without the --no-scope-hoist option?

Here's the most minimal repro I could throw that matches my setup with parcel, solid & your router. https://github.com/amoutonbrady/parcel-solid-router-bug

To reproduce, just clone it, install the dependencies & run the dev or build script. (I pinned the version to 0.1.3).

Thanks again!

rturnq commented 4 years ago

My configuration looks similar to what you have - just the minimal TypeScript setup. I assume you also have a .babelrc file with the typescript and solid presets otherwise Parcel assumes your JSX/TSX is React. I have not been able to build any Solid projects using Parcel 2 without the --no-scope-hoist, hopefully someone over there will take a look at the issue you filed soon.

I cloned your repo, did an npm i followed by npm run build and it built without any issues. I verified the version of the router npm installed really was 0.1.3. I added the missing .babelrc file, did an npm run dev and you project built and ran just fine.

I looked at Parcel 2's module resolution documentation found the following

Package entry fields

When scope hoisting is enabled, a bare specified (e.g. lodash) is resolved in this order (the first field that specified and points to ane existing file):

  • package.json#source
  • package.json#browser
  • package.json#module
  • package.json#main
  • index.{js, json}

Without scope hoisting however, main is preferred to module for better performance:

  • package.json#source
  • package.json#browser
  • package.json#main
  • package.json#module
  • index.{js, json}

It's not clear to me how specifying the import (bare? vs. destructured) changes things but it seems like they always use the source property from the package's package.json to start with. It also sounds like it is intended to be fault tolerant, "... and points to ane existing file" makes it sound like they check to see if a file exists at the path and move on to the next option if not.

So it seems to me, the file existence check is not working correctly for you. The only thing I can think of to explain the different behaviors we are seeing is environment:

Windows 10 node -v 12.16.1 npm -v 6.13.4

Either way, glad removing the source property resolved your issue.

rturnq commented 4 years ago

After doing a little more digging I suspect this is the reason we are seeing different outcomes: https://github.com/parcel-bundler/parcel/blob/c88f810a92638c9b57ff9529076d43cc7ca8e0d5/packages/utils/node-resolver-core/src/NodeResolver.js#L705-L712. If you have your packages symbolically linked then Parcel treats it as source code instead of a compiled dependency and retains the source field.

Removing the source field from my packages.json file seems to be the way to go.

amoutonbrady commented 4 years ago

Hey, sorry it took me a little bit to get back to you. First and foremost thanks for your time. I do indeed use Linux most of the time and find pnpm as my favorite package manager. It's main feature is indeed to make most modules in your node_modules, sym link to a global store on your computer.

Thanks as well for your help on the parcel issue!