abaga129 / sveltekit-adapter-iis

15 stars 5 forks source link

Dev dependencies copied as well as dependencies #16

Closed ablesea closed 9 months ago

ablesea commented 9 months ago

Can we make it so that only dependencies are copied? This could be opt in I am not sure if there are any cases where that could break code. Right now it will take longer for everything to install since its installing everything.

Basically you would just do this

export function createPackage() {
    if (!fs.existsSync("package.json")) {
        console.warn(`Could not find package.json at ${process.cwd()}/package.json`);
        return null;
    }

    const packageContent = fs.readFileSync("package.json");

    const packageJSON = JSON.parse(packageContent);

    packageJSON.devDependencies = {};

    return JSON.stringify(packageJSON);
}

then just write the content to the output folder.

ablesea commented 9 months ago

There may be a better way to implement this still

KraXen72 commented 9 months ago

afaik sveltekit has a somewhat unintuitive way of using the dependencies and devdependencies field: see this thread in svelte discord for more info. If you aren't in the svelte discord or the link is broken, let me know.

ieedan commented 9 months ago

That makes it kinda hard to just get rid of them ig. Pretty frustrating that you have to install dependencies post build anyways.

abaga129 commented 9 months ago

@ieedan what I found was that pretty much all dependencies for my projects could be installed as dev dependencies due to how sveltekit handles them. So in the end I had no need to rely on node_modules. But yes if for some reason you need some dependency that cant be bundled into your sveltekit build then it is a bit annoying. Probably the easiest method is to copy over your lockfile (which this adapter already does) and run your package manager in the deployed app directory.

ieedan commented 9 months ago

May be a good idea to add some documentation for this to suggest attempting to bundle all dependencies as dev dependencies so that you don't have to reinstall.

ablesea commented 9 months ago

So basically Sveltekit already includes all of the dev dependencies in the bundle so there is no need to reinstall them?

I think in this case my point would still stand that maybe we could add an optional flag like includeDevDependencies that would remove them from the copied package.json to prevent you from having to install them twice. It could be default true so it would be opt in.

I am not sure how many other use cases would benefit but from my initial testing I know mine would.

EDIT: I have a few dependencies that cry out for circular dependencies when they are included as dev dependencies (luxon, jsonwebtoken) so I still have to rely on installing node_modules on the host machine and removing the dev dependencies is the difference between installing 2 dependencies and 20

abaga129 commented 9 months ago

Hmm I wonder why that could cause circular dependencies. As an example. My package json used to have this

  "dependencies": {
    "@types/base-64": "^1.0.0",
    "@types/utf8": "^3.0.1",
    "axios": "^1.4.0",
    "base-64": "^1.0.0",
    "electron": "^25.4.0",
    "flatpickr": "^4.6.13",
    "svelte-select": "^5.6.1",
    "utf8": "^3.0.0"
  }

When I would publish this to IIS, I had to run yarn install to get all of those dependencies for use at runtime. After reading a bit about deps in Sveltekit, I moved them all to be devDependencies, and now I can omit node_modules from the final built app altogether.

ablesea commented 9 months ago

I am not sure why it would cause it jsonwebtoken depends on semver which is causing the error and luxon also gives an error on itself. I was able to move everything except for those.

ablesea commented 9 months ago

If this is something you're open to I can open a PR I think it would help a lot for those dependencies that can't be bundled as dev dependencies.

abaga129 commented 9 months ago

@ablesea Actually this may not even be necessary depending on your package manager. For example yarn has a --production switch that only installs dependencies. https://classic.yarnpkg.com/lang/en/docs/cli/install/#toc-yarn-install-production-true-false

Edit:

npm also supports it

npm install --production --no-optional

Bun

Pnpm

ieedan commented 9 months ago

Looks like npm supports with

npm install --only-production
# npm WARN config only Use `--omit=dev` to omit dev dependencies from the install.
npm install --omit=dev
abaga129 commented 9 months ago

I think it would still be good to document this in the readme somewhere as it's something many users are likely to run into at some point.

ieedan commented 9 months ago

So

npm

npm install --omit-dev

pnpm

pnpm install --prod
pnpm install --P

bun

bun install --production

yarn

yarn install --production=true
ieedan commented 9 months ago

++ Line 55 of README.md

  1. Install dependencies

You should try to bundle all your dependencies as dev dependencies so that you can skip this step however not all dependencies play nice. In this case you can install just the production dependencies using your preferred package manager.

npm

npm install --omit-dev

pnpm

pnpm install --P

yarn

yarn install --production=true

bun

bun install --production
KraXen72 commented 9 months ago

feel free to open pr adding this to readme if you find some time

ablesea commented 9 months ago

Yeah I will open one in a bit

ieedan commented 9 months ago

Sorry didn't get to this this evening I'll open one tomorrow morning