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

ESM dependencies are not resolved correctly during build #4692

Closed
1 task
Enteleform opened this issue Sep 9, 2022 · 20 comments
Closed
1 task

ESM dependencies are not resolved correctly during build #4692

Enteleform opened this issue Sep 9, 2022 · 20 comments
Assignees
Labels
- P2: has workaround Bug, but has workaround (priority)

Comments

@Enteleform
Copy link
Contributor

Enteleform commented Sep 9, 2022

What version of astro are you using?

1.1.7

Are you using an SSR adapter? If so, which one?

no

What package manager are you using?

npm

What operating system are you using?

Windows

Describe the Bug

This error occurs during build when multi-distribution (CJS, ESM) packages are imported.

> astro build

06:04:41 AM [build] output target: static
06:04:41 AM [build] Collecting build info...
06:04:41 AM [build] Completed in 18ms.
06:04:41 AM [build] Building static entrypoints...
06:04:42 AM [build] Completed in 700ms.

 building client 
Completed in 202ms.


 generating static routes 
 error   Named export 'atom' not found. The requested module 'solid-use' is a CommonJS module, which may not support all module.exports as named exports. 
  CommonJS modules can always be imported via the default export, for example using:
  
  import pkg from 'solid-use';
  const { atom } = pkg;
  
file:///D:/DemoProject/dist/entry.mjs?time=1662717882730:3
/* empty css                        */import { atom } from 'solid-use';
                                               ^^^^
SyntaxError: Named export 'atom' not found. The requested module 'solid-use' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'solid-use';
const { atom } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:123:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:189:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:528:24)
    at async generatePages (file:///D:/DemoProject/node_modules/astro/dist/core/build/generate.js:70:20)
    at async staticBuild (file:///D:/DemoProject/node_modules/astro/dist/core/build/static-build.js:67:5)
    at async AstroBuilder.build (file:///D:/DemoProject/node_modules/astro/dist/core/build/index.js:82:5)
    at async AstroBuilder.run (file:///D:/DemoProject/node_modules/astro/dist/core/build/index.js:123:7)
    at async build (file:///D:/DemoProject/node_modules/astro/dist/core/build/index.js:22:3)
    at async runCommand (file:///D:/DemoProject/node_modules/astro/dist/cli/index.js:138:14)

I included two demos in the reproduction repo, one using astro and one using solid-start. Both demos are minimally modified starter projects, they were updated to implement atom from solid-use instead of createSignal from solid-js.

The solid-start demo builds without issue. (they also use vite in their build process)

The astro demo throws the above error.

solid-use has both CJS & ESM distributions, with a properly configured package.json that should allow usage in either environment.

Link to Minimal Reproducible Example

https://github.com/enteleform-codesandbox/0015--Astro--ESM-Imports

Participation

  • I am willing to submit a pull request for this issue.
@matthewp
Copy link
Contributor

matthewp commented Sep 9, 2022

Is this fixed using vite: { ssr: { noExternal: [ 'solid-use' ] } } ?

@matthewp matthewp added the needs response Issue needs response from OP label Sep 9, 2022
@Enteleform
Copy link
Contributor Author

Enteleform commented Sep 10, 2022

@matthewp

Is this fixed using vite: { ssr: { noExternal: [ 'solid-use' ] } } ?

This seems to work 👍. Do you know why it's required for astro but not other vite projects? I just added another demo to the repro, vite's vanilla-ts starter. It also builds fine with solid-use without any explicit configuration.

Also, once I got past that error I ran into another one which seems to be caused by improper hoisting in the entry.mjs bundle. This one might be harder to make a repro for since it's the result of bundling multiple nested local dependencies, but I have built other vite/webpack projects using those same libraries without issue.

> astro build

(!) Could not auto-determine entry point from rollupOptions or html files and there are no explicit optimizeDeps.include patterns. Skipping dependency pre-bundling.
ts-toolbelt doesn't appear to be written in CJS, but also doesn't appear to be a valid ES module (i.e. it doesn't have "type": "module" or an .mjs extension for the entry point). Please contact the package author to fix.
05:16:48 AM [build] output target: static   
05:16:48 AM [build] Collecting build info...
05:16:48 AM [build] Completed in 25ms.      
05:16:48 AM [build] Building static entrypoints...
Use of eval is strongly discouraged, as it poses security risks and may cause issues with minification
05:16:51 AM [build] Completed in 2.98s.

 building client 

PWA v0.12.6
mode      generateSW
precache  16 entries (94.31 KiB)
files generated
  dist\sw.js
  dist\workbox-4d7aac67.js      
Completed in 5.36s.       


 generating static routes 
 error   Cannot access 'File' before initialization
ReferenceError: Cannot access 'File' before initialization
    at file:///D:/DemoProject/dist/entry.mjs?time=1662801417180:3288:28
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:528:24)
    at async generatePages (file:///D:/DemoProject/node_modules/astro/dist/core/build/generate.js:70:20) 
    at async staticBuild (file:///D:/DemoProject/node_modules/astro/dist/core/build/static-build.js:67:5)
    at async AstroBuilder.build (file:///D:/DemoProject/node_modules/astro/dist/core/build/index.js:82:5)
    at async AstroBuilder.run (file:///D:/DemoProject/node_modules/astro/dist/core/build/index.js:123:7) 
    at async build (file:///D:/DemoProject/node_modules/astro/dist/core/build/index.js:22:3)
    at async runCommand (file:///D:/DemoProject/node_modules/astro/dist/cli/index.js:138:14)
npm ERR! Lifecycle script `build` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: [email protected] 
npm ERR!   at location: D:\DemoProject 

 *  The terminal process "C:\WINDOWS\System32\cmd.exe /d /c npm run build" terminated with exit code: 1. 

@matthewp
Copy link
Contributor

If I had to guess, this line is probably causing it: https://github.com/withastro/astro/blob/main/packages/integrations/solid/src/index.ts#L42

can you try commenting that out in your local node_modules and see if it fixes any of those issues?

@Enteleform
Copy link
Contributor Author

can you try commenting that out in your local node_modules and see if it fixes any of those issues?

Just tried it, still ran into the original issue.

@ruhollahh
Copy link

ruhollahh commented Sep 10, 2022

I ran into this problem with the @codesandbox/sandpack-react package.
and this didn't help:

Is this fixed using vite: { ssr: { noExternal: [ '@codesandbox/sandpack-react ' ] } } ?

@matthewp
Copy link
Contributor

@ruhollahh that's likely unrelated.

@ruhollahh
Copy link

ruhollahh commented Sep 11, 2022

@ruhollahh that's likely unrelated.

I downgraded the astro version from 1.1.7 to 1.0.0-beta.73 and removed this:
legacy: { astroFlavoredMarkdown: true, }
my build problem was solved. 🤔
but now my md files don't work currectly.

@Enteleform
Copy link
Contributor Author

Just tried 1.0.0-beta.73 for the repro & still ran into the same issue.

@ruhollahh
Copy link

ruhollahh commented Sep 12, 2022

Okay I finally fixed the build problem by finding the the relevant package (in your case "solid-use") in node_modules folder and adding this to it's package.json file:
"type": "module"
also edited this:
"main": "dist/cjs/production/index.js"
to this:
"main": "dist/esm/production/index.js"
I tested this method in your Minimal Reproducible repo and it works, I was able to build without any errors.
no need for any downgrades or anything like that!

@Enteleform
Copy link
Contributor Author

Enteleform commented Sep 12, 2022

"main": "dist/esm/production/index.js"

There are two current workarounds that don't require altering affected packages:

Matthew's suggestion of:

{
  vite: {
    ssr: { 
      noExternal: ["solid-use"]
    }
  }
}

and also one that I had posted on the Discord server when I opened the issue:

{
  vite: {
    resolve: {
      alias: {"solid-use": "D:/DemoProject/node_modules/solid-use/dist/esm/production/index.js"}
    }
  }
}

The first one is better since it wouldn't require multiple entries for nested imports.

@matthewp matthewp added - P2: has workaround Bug, but has workaround (priority) and removed needs response Issue needs response from OP labels Sep 12, 2022
@matthewp
Copy link
Contributor

Is this a problem only in build but not in development mode?

@Enteleform
Copy link
Contributor Author

Enteleform commented Sep 12, 2022

Is this a problem only in build but not in development mode?

Yes, that's correct.

Same with the hoisting issue I mentioned in my second post, however, I haven't found a workaround for that issue yet.

@Enteleform
Copy link
Contributor Author

@matthewp

Seems like this might have something to do with monorepos/workspaces.

Just tried out the latest Completely Empty template from astro.new. It works fine if I use it on its own, but I get the following error when I tried running astro dev in my Yarn workspace:

> astro dev

file:///D:/DemoProject/node_modules/@astrojs/markdown-remark/dist/rehype-collect-headings.js:2  
import { toHtml } from "hast-util-to-html";
         ^^^^^^
SyntaxError: Named export 'toHtml' not found. The requested module 'hast-util-to-html' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'hast-util-to-html';
const { toHtml } = pkg;

  at ModuleJob._instantiate (node:internal/modules/esm/module_job:123:21)
  at async ModuleJob.run (node:internal/modules/esm/module_job:189:5)
  at async Promise.all (index 0)
  at async ESMLoader.import (node:internal/modules/esm/loader:528:24)

npm ERR! Lifecycle script `dev` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: @enteleform/[email protected] 
npm ERR!   at location: D:/DemoProject 

*  The terminal process "C:/WINDOWS/System32/cmd.exe /d /c npm run dev" terminated with exit code: 1. 

@Enteleform
Copy link
Contributor Author

Enteleform commented Sep 17, 2022

Actually, just noticed that that same issue (hast-util-to-html) broke one of my previously working projects.

I haven't changed anything in the project other than running yarn install on the workspace to install deps for other projects in the same workspace... Looking into it now...

@matthewp
Copy link
Contributor

Are you using shamefully-hoist=true in your .npmrc? That's the first thing I look for in monorepo issues.

@Enteleform
Copy link
Contributor Author

Are you using shamefully-hoist=true in your .npmrc? That's the first thing I look for in monorepo issues.

I think shamefully-hoist is just for pnpm. Good call though, got it working with this @ the yarn workspace root package.json:

"nohoist": [
  "**/astro",             "**/astro/**"
  "**/@vivliostyle/cli",  "**/@vivliostyle/cli/**",
]

Before that I was trying to go about it more granularly by targeting the clashing packages via yarn + resolutions, and npm + overrides, but neither of those seemed to work... 😑

@rishi-raj-jain
Copy link
Contributor

I think this is more of a rollup error.

@rishi-raj-jain
Copy link
Contributor

https://www.npmjs.com/package/@rollup/plugin-commonjs something like this shall help?

@Enteleform
Copy link
Contributor Author

Enteleform commented Sep 22, 2022

npmjs.com/package/@rollup/plugin-commonjs something like this shall help?

I tried that and a couple of Vite-specific ones without any luck.

 

I think this is more of a rollup error.

The issue doesn't occur in other Vite projects though, even a plain Vite starter - so it seems like the issue might have something to do with Astro's particular usage of Vite/Rollup.

@bluwy
Copy link
Member

bluwy commented Oct 13, 2022

solid-use is incorrectly packaged (https://publint.bjornlu.com/[email protected]), hence it doesn't work in node when Astro builds. I'd suggest opening an issue in their repo to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: has workaround Bug, but has workaround (priority)
Projects
None yet
Development

No branches or pull requests

5 participants