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

Issue with node shebang #109

Closed
MarceloAlves opened this issue May 22, 2019 · 4 comments · Fixed by #117
Closed

Issue with node shebang #109

MarceloAlves opened this issue May 22, 2019 · 4 comments · Fixed by #117
Labels
kind: bug Something isn't working

Comments

@MarceloAlves
Copy link

MarceloAlves commented May 22, 2019

Current Behavior

I could be doing something wrong here but creating a new "basic" project and adding the node shebang to index.ts and running yarn build --target node results in this error:

23:34:20 in tsdx-issue on  master is 📦  v0.1.0 via ⬢ v8.15.0 took 7s
➜ yarn build --target node
yarn run v1.15.2
warning package.json: No license field
$ tsdx build --target node
✓ Creating entry file 3.5 secs
Error when using sourcemap for reporting an error: Can't resolve original location of error.
Error when using sourcemap for reporting an error: Can't resolve original location of error.
Error when using sourcemap for reporting an error: Can't resolve original location of error.
Error when using sourcemap for reporting an error: Can't resolve original location of error.
(node:60781) UnhandledPromiseRejectionWarning: Error: Unexpected character '#' (Note that you need plugins to import files that are not JavaScript)
    at error (/Users/malves/Development/tmp/tsdx-issue/node_modules/rollup/dist/rollup.js:9236:30)
    at Module.error (/Users/malves/Development/tmp/tsdx-issue/node_modules/rollup/dist/rollup.js:12909:9)
    at tryParse (/Users/malves/Development/tmp/tsdx-issue/node_modules/rollup/dist/rollup.js:12825:16)
    at Module.setSource (/Users/malves/Development/tmp/tsdx-issue/node_modules/rollup/dist/rollup.js:13102:33)
    at Promise.resolve.catch.then.then.then (/Users/malves/Development/tmp/tsdx-issue/node_modules/rollup/dist/rollup.js:15924:20)
    at <anonymous>
(node:60781) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:60781) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Error when using sourcemap for reporting an error: Can't resolve original location of error.
✨  Done in 5.38s.

Expected behavior

Should build without an error as far as I know.

Suggested solution(s)

Not quite sure where the issue lies here. Spent a bit of time digging around to see if maybe I could figure it out but wasn't very successful.

Additional context

Sample Repo

Your environment

Software Version(s)
TSDX 0.5.11
TypeScript 3.4.5
Browser Chrome 74
npm/Yarn 1.15.2
Operating System macOS 10.14.4
@swyxio
Copy link
Collaborator

swyxio commented May 22, 2019

rollup seems to not handle this on purpose: rollup/rollup#235

we use @jaredpalmer's own shebang plugin: https://github.com/palmerhq/tsdx/blob/71817a33628983ee8a3adb4d10e852b5895d7a00/src/createRollupConfig.ts#L12 so it should work... will just leave it to you two to figure it out

@MarceloAlves
Copy link
Author

👋 @sw-yx thanks for chiming in here

I hadn't seen that issue in my searching! I'm thinking my project it might be a better route to build with tsc and stick to using tsdx when it comes to web-based packages.

I don't have enough experience with rollup to really dig in here. I did see this plugin which adds the shebang afterwards so that users don't have to add it themselves. I replaced the current one in createRollupConfig.ts and it seemed to work okay. But it looks like the preserve one should work too

Thanks again!

@hedgerh
Copy link
Contributor

hedgerh commented May 24, 2019

Took a look into this. What's the desired output? A shebang in each bundle, or one in dist/index.js? If the former, one way I got it to work is by changing the preserve shebang plugin's processEntry function to remove the shebang from the files before being processed and rewrite them to disk:

function processEntry(entry) {
		if (entry) {
			let contents = fs.readFileSync(entry, 'utf8');
			let matches = contents.match(/^\s*(#!.*)/);
			shebang = matches && matches[1] || false;

                        // new code:
                        if (!shebang) return;
                        contents = contents.replace(shebang, '')
       	                fs.writeFileSync(entry, contents)
		}
	}

not sure if there's a better way to do that, but it worked. on the flip side, it seems pretty easy to also automate it so they don't have to add their own.

lemme know what'd be preferred and i can implement it

Edit: dug into rollup's hooks more and realized there was a transform hook to do this.

@jaredpalmer
Copy link
Owner

You just want it in the entry file that has it. Not in every one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants