Snowpack: clean up and standardize cwd/root behavior (incl. JS API)

Created on 12 Nov 2020  Â·  10Comments  Â·  Source: snowpackjs/snowpack

Problem

When using the JS API (e.g. import { startDevServer } from 'snowpack';), the cwd option doesn‘t get passed through to the rest of snowpack, which poses a problem in a larger project. Consider the following scenario:

├── client/
│   ├── snowpack.config.js
│   └── src/
└── server/

Say a Snowpack project lives in client/, and inside snowpack.config.js, all the mounted folders point relatively within client/. Everything works fine when you‘re in that folder.

But if you import 'snowpack' inside server/, Snowpack can‘t find anything because it tries to resolve server/src/ which doesn‘t exist.

Proposed Solution

Fix the cwd option to all the Node API functions, like so:

startDevServer({
  cwd: __dirname
})

Currently, there is a cwd parameter, however, it doesn‘t get passed to the rest of the application. All the top-level methods should accept the cwd command, and this should override process.cwd() when it’s set:

  • snowpack.startDevServer()
  • snowpack.getUrlForFile()

When not set, it should fall back to its current value, process.cwd().

good first issue help wanted

All 10 comments

If alright, I'd like to take up and work on this issue!

@thescripted Sure! Please take a swing at it 🙂

@thescripted Drew and I were just talking about this, and I think we want to tackle this as a larger "what is the root in a Snowpack project?" Currently it is the process.cwd(), but for programatic usage (including our own testing needs) we want to turn this into a larger "root"/"resolveFrom" concept that sets the root of your project AND defaults to where your closest snowpack.config.js file lives.

We'd still love the help so if you're still interested in tackling, please let us know and we can provide guidance! The original post is still a part of this work (honor the "cwd" value in the JS API, and then rename it to "root" later). But otherwise, no worries if feels too big to bite off now.

I'm very interested in this, but I am stretched thin between my commitment to other work and my lack of knowledge in this space (and why I haven't updated much in this issue). This might be too big for me at the moment.

If, down the road, this or related issue gets broken down into multiple PRs I believe I could help out. Would also still love to find ways to contribute in general as well!

np, appreciate the offer to help! We're looking to get this out as a breaking change in v3, so @drwpow or @matthewp may be tackling sooner

@FredKSchott Would love to give it a shot if this is still open!

Here's what I accomplished so far:
https://github.com/februarycat/snowpack/tree/Replace-process-cwd-with-configurable-root-option

I'm not 100% sure it's complete as I didn't have much time today to test it all. When I have some free time I'll check if I can replace the rest of the cwd instances in the code.

I'll be waiting to hear your thoughts on whether this is a good start for the refactor!

Awesome! I may actually be taking over this initiative instead of @drwpow or @matthewp, in which case I'd love to help you with your PR! I'll take a look later today and can provide feedback.

Can you create a draft PR of your branch? It will be easier for me to check out and leave feedback that way

Thanks for the PR @februarycat! I'm going to mark this issue as done now that #1928 is done.

There's still some work to do to resolve relative paths from the location of the loaded file itself, but not directly related to the new root config.

Was this page helpful?
0 / 5 - 0 ratings