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

Create-Svelte - Fixing sucrase issue while running build script #1000

Conversation

rafaelcamaram
Copy link
Contributor

@rafaelcamaram rafaelcamaram commented Apr 13, 2021

I was trying to run the create-svelte CLI locally but I was not able to do it with success. I realized that we got the #989 merged and, with this, we were not able to run npm run build in order to build the dist folder (and then run the CLI to set a project).

import { transform } from 'sucrase';
         ^^^^^^^^^
SyntaxError: The requested module 'sucrase' is expected to be of type CommonJS, which does not support named exports. CommonJS modules can be imported by importing the default export.
For example:
import pkg from 'sucrase';
const { transform } = pkg;

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint

@rafaelcamaram rafaelcamaram changed the title Create-Svelte - Fixing sucrase issue in order get the build script working fine Create-Svelte - Fixing sucrase issue while running build script Apr 13, 2021
@benmccann
Copy link
Member

PR 1000!! Congrats! 😄

@Rich-Harris
Copy link
Member

thanks — can you fix the lint errors please? (note that i just pushed a commit to this branch — we use uppercase sparingly in these parts :)

FYI the reason this failed locally for you is probably that you're on an ancient version of Node. Since 12.20 or something like that, named imports from CommonJS modules are possible in the majority of cases

@Conduitry
Copy link
Member

I don't think there's anything that actually needs fixing here. If there was flakiness in some part of the automated build/publish, we should just try to run that again.

Locally, on latest master, I'm able to run pnpm -r build without error, and I'm able to manually run bin.js to create a new starter app from the template, and when I run npm pack in create-svelte to double-check which files would be getting published to npm, it contains precisely the files I would expect.

What's being changed in this PR does seem to just be something that's already supported in later versions of Node.

@Conduitry
Copy link
Member

I've opened #1001 to hopefully resolve the problem that was preventing the automatic publishes from happening.

@benmccann
Copy link
Member

Working version is published now. Thanks for pointing out the issue

@delhanty
Copy link

delhanty commented May 16, 2021

thanks — can you fix the lint errors please? (note that i just pushed a commit to this branch — we use uppercase sparingly in these parts :)

FYI the reason this failed locally for you is probably that you're on an ancient version of Node. Since 12.20 or something like that, named imports from CommonJS modules are possible in the majority of cases

Just a note for anybody else that follows me (also tag future self "mentioned" @delhanty): make sure you have at least the latest version of Node 14 installed.

I ran into this import { transform } from 'sucrase' problem with Node 14.6 and it went away when I upgraded to Node 14.17.

Edit: looks like the minimum versions are v12.20 and v14.13.1. I will make a pull request to document this in the README.

@dandv @dfabulich @kathy-ems Node.js 14.13.0 added support for named CJS exports, see #35249 and this change has also been backported to Node.js v12.20.0.

nodejs/node#32137 (comment)

delhanty pushed a commit to delhanty/kit that referenced this pull request May 17, 2021
sveltejs#1000).

See my comment

sveltejs#1000 (comment)

references

>Node.js 14.13.0 added support for named CJS exports, see #35249 and this change has also been backported to Node.js v12.20.0.

nodejs/node#32137 (comment)

and

>Yep, it was fixed after downloading 14.13.1, thank you

nodejs/node#35249 (comment)
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.

5 participants