Fix popperjs import for SvelteKit #356

benmccann · 2021-08-11T04:10:42Z

Closes #463
Closes #474
Closes #517

This updates @popperjs/core to the latest version, which no longer uses process.env, so it will work in the REPL

Import popper the normal way. Then Vite can use the CJS version on the server-side and ESM version on the client-side. Forcing it to always be ESM breaks SSR with the latest SvelteKit. See vitejs/vite#4569 / sveltejs/kit#2161

bestguy · 2021-08-11T14:12:00Z

Hi, thanks. Popper was giving troubles in Sapper and svelte.repl without this though due to use of process.env in their default distribution... I'll need to take a look at if this still works there, but if not we'd need some other resolution for SvelteKit.

bestguy · 2021-08-11T14:15:57Z

Related to this issue: #250

benmccann · 2021-08-11T17:22:49Z

Two wrongs don't make a right 😄 We should fix it in popper core. I sent a PR there: floating-ui/floating-ui#1342

Sapper's deprecated at this point. In the short-term, I'd rather have it working in SvelteKit and broken in the REPL and Sapper than vice versa since SvelteKit will be the main way people will be consuming the library. Hopefully in the medium-term we can get popper.js itself fixed rather than trying to work around the issue

bestguy · 2021-08-11T20:10:55Z

Agreed popper would be great to fix, but I and other users use sapper and repl frequently and would rather not break that. I'll see if there's something we can do. I had debated forking popper as well to remove, but eh.

vfilatov · 2021-08-13T15:59:40Z

Agreed popper would be great to fix, but I and other users use sapper and repl frequently and would rather not break that. I'll see if there's something we can do. I had debated forking popper as well to remove, but eh.

Actually SvelteStrap works fine with SvelteKit now if you do import from root (a regular way), no changes needed.

bestguy · 2021-08-13T16:23:23Z

Ah thanks @vfilatov , does sveltekit not need the src imports for SSR such as sapper needed?

vfilatov · 2021-08-13T16:38:13Z

Ah thanks @vfilatov , does sveltekit not need the src imports for SSR such as sapper needed?

After SvelteKit v1.0.0-next.146 no, it does not.

  import {
    Alert,
    Badge,
    Button,

    Modal,
    ModalHeader,
    ModalBody,
    ModalFooter
  } from 'sveltestrap';

benmccann · 2021-08-13T17:20:34Z

Ah, the example I was debugging did import from src. I didn't even notice that. sveltejs/kit#2161 (comment)

I do still think this is a good change, but I guess we can wait until popper itself is fixed

In the meantime, I sent a PR to update the readme: #359

bato3 · 2022-03-25T10:43:54Z

also in https://github.com/bestguy/sveltestrap/blob/master/src/Tooltip.svelte#L3
https://github.com/bestguy/sveltestrap/blob/master/src/popper.js#L1

avafloww · 2022-04-08T03:07:27Z

Bump, would love to see this merged!

benmccann · 2022-04-13T03:52:23Z

@bestguy my PR floating-ui/floating-ui#1342 was merged to fix popper-core. Could this be merged now as a result?

dominikg · 2022-04-13T06:15:35Z

looks like they reverted it floating-ui/floating-ui#1362 (would have been a breaking change)

is it an option to switch to floatingui dom?

edit: there's another PR trying to add export maps to popper again floating-ui/floating-ui#1526

benmccann · 2022-04-13T23:52:35Z

It looks like it's been fixed on the main branch for an eventual v3 floating-ui/floating-ui#1502 and that floating-ui/floating-ui#1526 would backport it to 2.x

Anyway, I wonder if the original reason for not merging this still applies. It was said that we wanted to prioritize Sapper over SvelteKit, but that was over a year ago. Now SvelteKit has almost 2x the usage of Sapper and so I would think that it would be better to prioritize SvelteKit over Sapper. Would that logic make sense to you @bestguy? If you're still on Sapper by chance, I'd be happy to advice on a migration to SvelteKit

ynshung · 2022-04-18T04:42:34Z

Is there any alternative or workaround that can be used before this PR get merged?

Edit: I found that the fork by laxadev/sveltestrap is working fine without any errors. I install its dependencies by running npm i git+https://github.com/laxadev/sveltestrap.git.

king612 · 2022-04-18T16:49:53Z

I am experiencing the same issue with Sveltestrap. It actually prevents me from running SvelteKit. I'd really like to use bootstrap for svelte app dev. Any workarounds would be appreciated, and we really need a proper fix to this. Thanks.

Cannot use import statement outside a module
/Users/john/projects/dems/node_modules/@popperjs/core/dist/esm/popper.js:1
import { popperGenerator, detectOverflow } from "./createPopper.js";
^^^^^^

SyntaxError: Cannot use import statement outside a module
at Object.compileFunction (node:vm:352:18)
at wrapSafe (node:internal/modules/cjs/loader:1032:15)
:

With this package.json:

{
"name": "dems",
"version": "0.0.1",
"scripts": {
"dev": "svelte-kit dev",
"build": "svelte-kit build",
"package": "svelte-kit package",
"preview": "svelte-kit preview",
"prepare": "svelte-kit sync"
},
"devDependencies": {
"@sveltejs/adapter-auto": "next",
"@sveltejs/kit": "next",
"svelte": "^3.47.0"
},
"type": "module",
"dependencies": {
"@fontsource/fira-mono": "^4.5.0",
"@lukeed/uuid": "^2.0.0",
"bootstrap": "^5.1.3",
"cookie": "^0.4.1",
"sveltestrap": "^5.9.0"
}
}

vfilatov · 2022-04-18T17:07:55Z

I use manual add

{
  "name": "@popperjs/core",
  "type": "module", // <<<<<<<<<<<<<
  "version": "2.11.5",

to ./node_modules/.pnpm/node_modules/@popperjs/core/package.json
as a temporary workaround

I am experiencing the same issue with Sveltestrap. It actually prevents me from running SvelteKit. I'd really like to use bootstrap for svelte app dev. Any workarounds would be appreciated, and we really need a proper fix to this. Thanks.

Cannot use import statement outside a module /Users/john/projects/dems/node_modules/@popperjs/core/dist/esm/popper.js:1 import { popperGenerator, detectOverflow } from "./createPopper.js"; ^^^^^^

SyntaxError: Cannot use import statement outside a module at Object.compileFunction (node:vm:352:18) at wrapSafe (node:internal/modules/cjs/loader:1032:15) :

With this package.json:

{ "name": "dems", "version": "0.0.1", "scripts": { "dev": "svelte-kit dev", "build": "svelte-kit build", "package": "svelte-kit package", "preview": "svelte-kit preview", "prepare": "svelte-kit sync" }, "devDependencies": { "@sveltejs/adapter-auto": "next", "@sveltejs/kit": "next", "svelte": "^3.47.0" }, "type": "module", "dependencies": { "@fontsource/fira-mono": "^4.5.0", "@lukeed/uuid": "^2.0.0", "bootstrap": "^5.1.3", "cookie": "^0.4.1", "sveltestrap": "^5.9.0" } }

Due to this issue bestguy/sveltestrap#356

skornel02 · 2022-05-22T07:36:55Z

I ran into the same issue. I use SvelteKit (1.0.0-next.338 as of writing) and my project could not start up because of the above detailed issue.

Is there any alternative or workaround that can be used before this PR get merged?

Edit: I found that the fork by laxadev/sveltestrap is working fine without any errors. I install its dependencies by running npm i git+https://github.com/laxadev/sveltestrap.git.

The fork suggested by @ynshung worked flawlessly for me.

kbsali · 2022-05-30T12:46:42Z

I have compiled some of the PRs opened into one + updated the dependencies (patch & minor, not major)

the PR : fix popper issue, fix failing tests & bump dependencies #488
the corresponding fork : https://github.com/ClapClap-app/sveltestrap/
to use it : npm i -D github:ClapClap-app/sveltestrap

As proposed in another issue, we should organize ourselves to maintain an "official" fork :)

bestguy · 2022-10-02T03:12:33Z

Everyone, and @benmccann @kbsali , I really appreciate the PRs and desire to fix this for SvelteKit, but none of these fixes and forks changing to '@popperjs/core' correct the root issue, which is why the PRs to change popper imports have not been merged after so long.

I've tried the same in past as I mentioned, but I've been unwilling to break the Svelte REPL. This is not deprecated AFAIK, and I'm a heavy user with Sveltestrap. (I know Sapper is deprecated so I wont worry about that one any longer).

Upgrading to Floating UI won't fix this either, as they continue the process.env usage: https://floating-ui.com/docs/getting-started#package-entry-points , and they have not accepted PRs in past to change this. It's really frustrating but that is their choice.

Only solutions I can think of is:

use something other than popper or floating-ui (please suggest if you are aware of libaries)
Fork popper and remove process.env and use.
Change Svelte REPL rollup config with the workaround popper suggests above. (I can try and PR/issue there, but that repo seems even less responsive than I am! 😅)

I will gladly take any help or ideas that fixes this if it works on svelte.dev
I really do not want to break this!

I'll see if https://github.com/Simonwep/nanopop could be used without breaking changes instead of popper.

benmccann · 2023-03-24T18:29:28Z

Ah, I hadn't realized the REPL issue. Thanks for clarifying.

I've mentioned the REPL issue to the other Svelte maintainers. I'm of two minds as to whether we should makes changes there. On the one hand, there are some libraries that won't work if we don't polyfill process.env. On the other hand, we should discourage process.env usage. Perhaps there's some third option like an advanced mode to let you edit the Rollup config. It's not a simple case to decide what is ideal. We can certainly review a PR if we decide, but deciding is not easy.

It's not clear to me that floating-ui has rejected replacing process.env. It was the PR author who closed the PR (floating-ui/floating-ui#2029). This would be the best solution, so I think we should pursue it still. I think we just need to do the work to make sure it works in all cases and the maintainers are comfortable that it works in all cases.

In the meantime, usage with either the REPL or SvelteKit is impacted. I'm a bit surprised about picking the REPL over SvelteKit since many more users will be using SvelteKit. Also, there is currently a workaround implemented which hides the true issue contributing to it not being resolved yet. StackBlitz could be used for demos of this library rather than the REPL since there you have access to the Rollup config and can modify it as required.

atomiks · 2023-03-25T22:06:30Z

I'm making PRs to remove these in both Popper and Floating UI. I don't think they're worth the trouble anymore, especially since the lib is usually used transitively.

benmccann · 2023-03-26T04:18:39Z

That's great. Once floating-ui/floating-ui#2296 is merged, I can bump this PR to use the latest version of Popper, which should make this library work everywhere

godmar · 2023-04-08T20:40:55Z

I'm surprised this is not higher priority. For me, it breaks my first attempt at using sveltestrap altogether.

The "hello world" style example:

<script>
    import { Button } from 'sveltestrap'
</script>
<Button>a bootstrap button</Button>

triggers this error.
Here's my package.json:

{
	"name": "svelte-app",
	"version": "0.0.1",
	"private": true,
	"scripts": {
		"dev": "vite dev",
		"build": "vite build",
		"preview": "vite preview",
		"lint": "prettier --plugin-search-dir . --check . && eslint .",
		"format": "prettier --plugin-search-dir . --write ."
	},
	"devDependencies": {
		"@sveltejs/adapter-auto": "^2.0.0",
		"@sveltejs/kit": "^1.5.0",
		"eslint": "^8.28.0",
		"eslint-config-prettier": "^8.5.0",
		"eslint-plugin-svelte3": "^4.0.0",
		"prettier": "^2.8.0",
		"prettier-plugin-svelte": "^2.8.1",
		"svelte": "^3.54.0",
		"vite": "^4.2.0"
	},
	"type": "module",
	"dependencies": {
		"@sveltejs/adapter-static": "^2.0.1",
		"bootstrap": "^5.2.3",
		"sveltestrap": "^5.10.0"
	}
}

ps: this workaround appears to work.

benmccann · 2023-05-26T18:41:31Z

@bestguy my PR was merged to Popper and I updated this PR to use the latest version. You should be able to merge this PR now to make sveltestrap work out-of-the-box with both SvelteKit and the REPL

MattPhantastic · 2023-07-30T03:58:41Z

Which versions in the devDependencies in package.json still work with Sveltestrap out-of-the-box?

  "devDependencies": {
    "@sveltejs/adapter-auto": "^?",
    "@sveltejs/kit": "^?",
    "@typescript-eslint/eslint-plugin": "^?",
    "@typescript-eslint/parser": "^?",
    "eslint": "^?",
    "eslint-config-prettier": "^?",
    "eslint-plugin-svelte": "^?",
    "prettier": "^?",
    "prettier-plugin-svelte": "^?",
    "svelte": "^?",
    "svelte-check": "^?",
    "tslib": "^?",
    "typescript": "^?",
    "vite": "^?"
  }

benmccann · 2023-07-30T04:07:28Z

@bestguy are there any concerns you have that are keeping you from merging this?

benmccann · 2023-08-04T17:07:32Z

@bestguy @gary-mycase just checking in here again. To summarize this uses the latest popper which includes floating-ui/floating-ui#2296, so will now work out-of-the-box in both SvelteKit and the REPL

bestguy · 2023-08-04T23:33:30Z

Thanks @benmccann , let me test over weekend with a pre-release, will try and get this out asap with the 5.3 release

benmccann · 2023-08-05T03:28:19Z

Hurray! Thank you so much!

laxadev · 2023-08-06T11:31:09Z

@bestguy I added one more PR #564

Fix popperjs import for SvelteKit

b41769f

This was referenced Aug 11, 2021

Vite prebundling breaks SSR vitejs/vite#4569

Closed

SvelteKit v1.0.0-next.146 SyntaxError: Cannot use import statement outside a module sveltejs/kit#2161

Closed

kbsali mentioned this pull request Mar 24, 2022

Error when evaluating SSR module /node_modules/sveltestrap/src/popper.js #463

Closed

benmccann mentioned this pull request Apr 13, 2022

Cannot use import module statement outside a module when importing and using sveltestrap component on sveltekit sveltejs/kit#4603

Closed

benmccann mentioned this pull request Apr 13, 2022

Cannot use import statement outside a module sveltejs/kit#4504

Closed

jonjes mentioned this pull request May 3, 2022

CJS and ESM issue keenethics/svelte-notifications#53

Closed

evrys added a commit to sprachy/sprachy that referenced this pull request May 22, 2022

Build fixes; use fork of sveltestrap temporarily

2282b0b

Due to this issue bestguy/sveltestrap#356

8lall0 mentioned this pull request Aug 1, 2022

Apologies for delayed response #494

Open

cjbruin23 approved these changes Mar 24, 2023

View reviewed changes

elkorman15 approved these changes Aug 1, 2023

View reviewed changes

benmccann added 2 commits August 4, 2023 10:01

Merge remote-tracking branch 'upstream/master' into patch-1

de02c90

update to latest popper

0cc7b50

benmccann force-pushed the patch-1 branch from 3952da2 to 0cc7b50 Compare August 4, 2023 17:03

bestguy approved these changes Aug 4, 2023

View reviewed changes

bestguy merged commit 108b461 into bestguy:master Aug 4, 2023

benmccann deleted the patch-1 branch August 5, 2023 03:28

laxadev mentioned this pull request Aug 6, 2023

Fix popperjs import for SvelteKit Patch-2 #564

Closed

benmccann mentioned this pull request Aug 7, 2023

fix: make package to work correctly with Vite #566

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix popperjs import for SvelteKit #356

Fix popperjs import for SvelteKit #356

benmccann commented Aug 11, 2021 •

edited

bestguy commented Aug 11, 2021 •

edited

bestguy commented Aug 11, 2021

benmccann commented Aug 11, 2021

bestguy commented Aug 11, 2021

vfilatov commented Aug 13, 2021

bestguy commented Aug 13, 2021

vfilatov commented Aug 13, 2021

benmccann commented Aug 13, 2021

bato3 commented Mar 25, 2022 •

edited

avafloww commented Apr 8, 2022

benmccann commented Apr 13, 2022

dominikg commented Apr 13, 2022 •

edited

benmccann commented Apr 13, 2022

ynshung commented Apr 18, 2022 •

edited

king612 commented Apr 18, 2022

vfilatov commented Apr 18, 2022 •

edited

skornel02 commented May 22, 2022

kbsali commented May 30, 2022

bestguy commented Oct 2, 2022 •

edited

benmccann commented Mar 24, 2023

atomiks commented Mar 25, 2023

benmccann commented Mar 26, 2023 •

edited

godmar commented Apr 8, 2023 •

edited

benmccann commented May 26, 2023

MattPhantastic commented Jul 30, 2023

benmccann commented Jul 30, 2023

benmccann commented Aug 4, 2023

bestguy commented Aug 4, 2023

benmccann commented Aug 5, 2023

laxadev commented Aug 6, 2023

Fix popperjs import for SvelteKit #356

Fix popperjs import for SvelteKit #356

Conversation

benmccann commented Aug 11, 2021 • edited

bestguy commented Aug 11, 2021 • edited

bestguy commented Aug 11, 2021

benmccann commented Aug 11, 2021

bestguy commented Aug 11, 2021

vfilatov commented Aug 13, 2021

bestguy commented Aug 13, 2021

vfilatov commented Aug 13, 2021

benmccann commented Aug 13, 2021

bato3 commented Mar 25, 2022 • edited

avafloww commented Apr 8, 2022

benmccann commented Apr 13, 2022

dominikg commented Apr 13, 2022 • edited

benmccann commented Apr 13, 2022

ynshung commented Apr 18, 2022 • edited

king612 commented Apr 18, 2022

vfilatov commented Apr 18, 2022 • edited

skornel02 commented May 22, 2022

kbsali commented May 30, 2022

bestguy commented Oct 2, 2022 • edited

benmccann commented Mar 24, 2023

atomiks commented Mar 25, 2023

benmccann commented Mar 26, 2023 • edited

godmar commented Apr 8, 2023 • edited

benmccann commented May 26, 2023

MattPhantastic commented Jul 30, 2023

benmccann commented Jul 30, 2023

benmccann commented Aug 4, 2023

bestguy commented Aug 4, 2023

benmccann commented Aug 5, 2023

laxadev commented Aug 6, 2023

benmccann commented Aug 11, 2021 •

edited

bestguy commented Aug 11, 2021 •

edited

bato3 commented Mar 25, 2022 •

edited

dominikg commented Apr 13, 2022 •

edited

ynshung commented Apr 18, 2022 •

edited

vfilatov commented Apr 18, 2022 •

edited

bestguy commented Oct 2, 2022 •

edited

benmccann commented Mar 26, 2023 •

edited

godmar commented Apr 8, 2023 •

edited