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

Use correct paths in package.json browser field #5174

Merged
merged 1 commit into from May 22, 2022
Merged

Use correct paths in package.json browser field #5174

merged 1 commit into from May 22, 2022

Conversation

ghost
Copy link

@ghost ghost commented May 12, 2022

The seeder entry is the only one really required to easily build knex for
the browser without shimming fs, but I updated the migrations path to
match.

However, there's also lib/migrations/seed/seed-stub.js, which actually
seems to provide errors when attempting to use the seed functionality in
a browser, but isn't actually used anywhere

I'm not exactly sure which way is intended.

If the seed stub is intended, then I'll change it to use that, otherwise it should probably be removed.

The seeder entry is the only one really required to easily build knex for
the browser without shimming `fs`, but I updated the migrations path to
match.

However, there's also `lib/migrations/seed/seed-stub.js`, which actually
seems to provide errors when attempting to use the seed functionality in
a browser, but isn't actually used anywhere

I'm not exactly sure which way is intended.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.282% when pulling cd3a7cc on jrobeson:fix-browser-package-json into e2516cd on knex:master.

@kibertoad kibertoad merged commit 3a874ec into knex:master May 22, 2022
@kibertoad
Copy link
Collaborator

Thank you!

@ghost
Copy link
Author

ghost commented May 22, 2022

but what about the seed stub?

@kibertoad
Copy link
Collaborator

It was introduced a long time ago, I'm not entirely sure. Probably updating it to be on a safe side makes sense.

@ghost
Copy link
Author

ghost commented May 22, 2022

what do you mean updating it? It should either be used in place of the noop stub or deleted shouldn't it?

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

Successfully merging this pull request may close these issues.

None yet

2 participants