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

doesn't work when bundled #79

Open
OmgImAlexis opened this issue Oct 25, 2021 · 12 comments
Open

doesn't work when bundled #79

OmgImAlexis opened this issue Oct 25, 2021 · 12 comments

Comments

@OmgImAlexis
Copy link

OmgImAlexis commented Oct 25, 2021

Because of this line the sql file ends up checking the relative location in this case it's dist instead of node_modules. Maybe inline this so it can be bundled? Or allow using the one supplied by the user.

path.join(__dirname, "migrations/0_create-migrations-table.sql"),

@ThomWright
Copy link
Owner

Hi, thanks for the issue.

By "bundled", do you mean you are merging the files in this library together into e.g. a single file?

Can I ask why?

@OmgImAlexis
Copy link
Author

Yep, when using something like rollup I can deploy my app in a single file.

With this issue I'm currently needing to install this dep at runtime so I can use the version in node_modules.

@ThomWright
Copy link
Owner

Thanks.

I think I still don't understand why this is desirable! Is it to make distribution easier? Just give someone a single .js file and tell them to run it with Node?

Does e.g. rollup have docs on how to handle __dirname? I expect this could be a common problem.

@OmgImAlexis
Copy link
Author

It's usually done for embedded apps and/or serverless like aws lambda. I'm honestly surprised this hasn't come up in the past.

Usually with rollup the __dirname will be rewritten to the base directory, thing is that's not going to help in this case as the file isn't in the base directory it's in node_modules.

I have a feeling the new way of getting __dirname might work in this case.

const __dirname = path.dirname(new URL(import.meta.url).pathname);

@mishabruml
Copy link

hey, any movement on this? I am getting the same error when bundling with esbuild

client = await pool.connect();
const migrationsDir = path.resolve('resources/migrations');
console.log(migrationsDir);
await migrate({ client }, migrationsDir);
/usr/app/resources/migrations # result of console.log(migrationsDir). Directory exists on docker image at expected location

error:   ┏ Error running db migrations ENOENT: no such file or directory, open '/usr/app/migrations/0_create-migrations-table.sql' - Offending file: '0_create-migrations-table.sql'.

error:   ┃ [ 1] Error: ENOENT: no such file or directory, open '/usr/app/migrations/0_create-migrations-table.sql' - Offending file: '0_create-migrations-table.sql'.

The path that the library tries to resolve is incorrect, its missing the /resources/
/usr/app/migrations/0_create-migrations-table.sql is an invalid path
/usr/app/resources/migrations/0_create-migrations-table.sql is a valid path

When running locally via ts-node there is no issues

@ThomWright
Copy link
Owner

How have other libraries solved this? I expect other libraries read files in similar ways and have had the same problem.

I'm open to solutions, but I don't have much time at the moment to think about, research and test this.

@mishabruml
Copy link

How have other libraries solved this? I expect other libraries read files in similar ways and have had the same problem.

I'm open to solutions, but I don't have much time at the moment to think about, research and test this.

Have a look through evanw/esbuild#859

@mishabruml
Copy link

You could try e.g. path.join(process.cwd()) from evanw/esbuild#859 (comment)

@ThomWright
Copy link
Owner

You could try e.g. path.join(process.cwd()) from evanw/esbuild#859 (comment)

I would be surprised if that worked in a normal Node.js/NPM environment.

Perhaps you would like to try it?

I see you're also based in Bristol, hi!

@mishabruml
Copy link

You could try e.g. path.join(process.cwd()) from evanw/esbuild#859 (comment)

I would be surprised if that worked in a normal Node.js/NPM environment.

Perhaps you would like to try it?

I will try and get some time

I see you're also based in Bristol, hi!

I was gonna say the same thing 😀 Wasson me babber!

@khromov
Copy link

khromov commented Jan 26, 2023

👋 Also running into this issue. As for why we want to bundle postgres-migrations:

We're developing a SvelteKit application. SvelteKit is a meta framework that automates building isomorphic applications, compiled to both frontend and backend bundles using Vite. I'm then packaging the resulting bundles into a Docker image. If I can't bundle postgres-migrations I'd have to include my entire node_modules folder into the Docker image which makes it about 4x larger than if I just include the bundled version.

Because postgres-migrations will always run on the server bundle, I can actually fetch the path there using:

import path from 'path';
import { fileURLToPath } from 'url';

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);

However since postgres-migrations just reads the global __dirname when I try to run pgMigrations.migrate(...) I get:

ReferenceError: __dirname is not defined`.

This happens at this line of code that includes the "0" migration. If only this line could be handled in a different way (for example, by inlining the initial zero migration) then this whole package should work fine even when bundling!

@khromov
Copy link

khromov commented Feb 5, 2023

I made a PR with a workaround for this issue. In short it involves copying the initial migration to your local migrations folder to skip the use of __dirname.

You can read more here:
#99

This will hopefully get merged but if you want to use my fork temporarily you can change your package.json file to look like this and then run npm i:

"postgres-migrations": "https://github.com/khromov/postgres-migrations",

PS.
Keep in mind the fork doesn't have the dist/ files so you need TypeScript support in your bundler and you will need to import the module like this:

import { migrate } from 'postgres-migrations/src/index';

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

No branches or pull requests

4 participants