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

Fix esbuild module resolve and missing parameter passing #165

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

fritx
Copy link
Collaborator

@fritx fritx commented Jan 15, 2024

Steps to reproduce the bug:

cd create-www/simple-crm
echo {} > package.json
npm install --dev esbuild
node $(which nue)
# ok
node $(which nue) build --production
# error: Bundler not found. Please use Bun or install esbuild

1. Fixed esbuild module resolve and missing parameter passing

  • introduced import-meta-resolve, because import.meta.resolve doesn't work for me
  • manually tested linking and nue build --production --esbuild under both node and bun

2. Added Testing and Linking guide to Contributing docs

I found I can only link by bun link or npm link to develop and test nuekit locally.

If you guys can do it via pnpm link, pls let me know :)

pnpm link+node $(which nue) (as documented in official docs) is not working:
WX20240115-124007

# /Users/fritx/Library/pnpm/nue
#!/bin/sh
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")

case `uname` in
    *CYGWIN*) basedir=`cygpath -w "$basedir"`;;
esac

if [ -z "$NODE_PATH" ]; then
  export NODE_PATH="/Users/fritx/g/nue/packages/nuekit/src/node_modules:/Users/fritx/g/nue/packages/nuekit/node_modules:/Users/fritx/g/nue/packages/node_modules:/Users/fritx/g/nue/node_modules:/Users/fritx/g/node_modules:/Users/fritx/node_modules:/Users/node_modules:/node_modules:/Users/fritx/Library/pnpm/global/5/.pnpm/node_modules"
else
  export NODE_PATH="/Users/fritx/g/nue/packages/nuekit/src/node_modules:/Users/fritx/g/nue/packages/nuekit/node_modules:/Users/fritx/g/nue/packages/node_modules:/Users/fritx/g/nue/node_modules:/Users/fritx/g/node_modules:/Users/fritx/node_modules:/Users/node_modules:/node_modules:/Users/fritx/Library/pnpm/global/5/.pnpm/node_modules:$NODE_PATH"
fi
if [ -x "$basedir/bun" ]; then
  exec "$basedir/bun"  "$basedir/global/5/node_modules/nuekit/src/cli.js" "$@"
else
  exec bun  "$basedir/global/5/node_modules/nuekit/src/cli.js" "$@"
fi

@fritx fritx force-pushed the docs-contributing branch 3 times, most recently from 3d78967 to 4a39fd7 Compare January 15, 2024 09:32
@fritx fritx changed the title Docs: Add local testing and linking guide WIP: Docs: Add local testing and linking guide Jan 15, 2024
@fritx fritx changed the title WIP: Docs: Add local testing and linking guide Add local testing and linking guide, fix esbuild module resolve Jan 15, 2024
@fritx fritx changed the title Add local testing and linking guide, fix esbuild module resolve WIP: Add local testing and linking guide, fix esbuild module resolve Jan 15, 2024
@fritx fritx changed the title WIP: Add local testing and linking guide, fix esbuild module resolve Fix esbuild module resolve and missing parameter passing Jan 15, 2024
@tipiirai tipiirai merged commit 73b931a into nuejs:master Jan 15, 2024
1 check passed
@tipiirai
Copy link
Contributor

Awesome/important fix again! Thanks so much.

Wonder if the import-meta-resolve module should be used on the findModule function as well. I use it to check for existence of lightningcss and stylus modules. Nuekit should start using lightningcss after calling bun add lightningcss on the project directory and when editing a CSS file. You should see a lightning icon on the console.

btw: would you like to have access rights to this repository for merging pull requests?

@fritx
Copy link
Collaborator Author

fritx commented Jan 16, 2024

Thanks, I'd love to help. But I'm just learning nuejs progressively. 😄
Although I have submitted some contributions, I actually have not yet understood the entire code base.
Its minimalism is awesome and open for anyone to learn and get in touch with.

Wonder if the import-meta-resolve module should be used on the findModule function as well

Sorry, I was missing there was already a findModule. Now I've tested under both Bun and Node.
Finding that the simple implementation of findModule is also working, while the path='lib/main.js' is required in Node.
(Looks like there are still a bit of environment differences between Bun and Node...)

try {
    if (!is_esbuild) return Bun
    // working
    // const esbuild_entry = await resolve('esbuild', `file://${process.cwd()}/`)
    // return await import(esbuild_entry)
    // also working
    const esbuild = await findModule('esbuild', 'lib/main.js') // required in Node
    if (!esbuild) throw new Error('Cannot find esbuild')
    return esbuild
  } catch (err) {
    throw 'Bundler not found. Please use Bun or install esbuild'
  }

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