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

feat: add basePath as config option #55

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

myabeaver
Copy link

@myabeaver myabeaver commented Jun 29, 2023

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix
[x] New feature
[ ] Other, please explain:

What changes did you make? (Give an overview)

  • added option to set basePath

Copy link
Owner

@aralroca aralroca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution @myasteiner 😊

Please add a test about this feature, you can use a similar example of this:

describe('nextTranslate', () => {
describe('nextTranslate -> pagesInDir', () => {
test('should detect correctly the appFolder and pagesFolder depending on the pagesInDir', () => {
fs.readdirSync.mockImplementationOnce(() => [])
const config = nextTranslate({})
expect(config.webpack({})).toEqual(
expect.objectContaining({

and check the final next config is without the nextTranslate.

src/index.ts Outdated Show resolved Hide resolved
@aralroca
Copy link
Owner

@myasteiner ping 🏓 🤗

@laneme
Copy link

laneme commented Jul 22, 2023

🤗 🏓

@mkbctrl
Copy link

mkbctrl commented Aug 6, 2023

Is it possible to takeover?

@aralroca
Copy link
Owner

aralroca commented Aug 8, 2023

Is it possible to takeover?

when the changes I have mentioned in the comments have been made. If we see that @myasteiner does not have time, we would implement it in another PR.

@myabeaver
Copy link
Author

Sorry, I would do that if the coming weeks I've nobody has time before.

@myabeaver myabeaver force-pushed the add-base-path-config-option branch 2 times, most recently from 876285a to 28bceae Compare August 24, 2023 12:39
@myabeaver myabeaver force-pushed the add-base-path-config-option branch from 28bceae to b2fcecf Compare August 29, 2023 14:13
@myabeaver
Copy link
Author

myabeaver commented Aug 29, 2023

Is it possible to takeover?

when the changes I have mentioned in the comments have been made. If we see that @myasteiner does not have time, we would implement it in another PR.

I've updated the PR does it look good for you? Btw, can we maybe also add an option to opt-out of getStaticProps per default? @aralroca

@jporsay
Copy link
Contributor

jporsay commented Sep 20, 2023

Hey @aralroca , would you mind taking a look at this PR? This is something that's a blocker for upgrading dependencies which bring very nice additions :)

@webpro
Copy link

webpro commented Oct 6, 2023

For the time being, I've released @webpro/[email protected]. Example CJS usage:

nextTranslate({ nextTranslate: { basePath: __dirname } })

Note that I've changed the implementation a bit compared to this PR (so you can pass the basePath and it won't be touched):

  let basePath =
    nextTranslate.basePath ??
    path.resolve(
      path.relative(pkgDir(), process.env.NEXT_TRANSLATE_PATH || '.')
    )

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.

6 participants