Fix popperjs import for SvelteKit

[benmccann]

Closes https://github.com/bestguy/sveltestrap/issues/463 Closes https://github.com/bestguy/sveltestrap/issues/474 Closes https://github.com/bestguy/sveltestrap/issues/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 https://github.com/vitejs/vite/issues/4569 / https://github.com/sveltejs/kit/issues/2161

[bestguy]

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]

Related to this issue: https://github.com/bestguy/sveltestrap/issues/250

[benmccann]

Two wrongs don't make a right :smile: We should fix it in popper core. I sent a PR there: https://github.com/popperjs/popper-core/pull/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]

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]

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]

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

[vfilatov]

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]

Ah, the example I was debugging did import from src. I didn't even notice that. https://github.com/sveltejs/kit/issues/2161#issue-965482733

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: https://github.com/bestguy/sveltestrap/pull/359

[bato3]

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]

Bump, would love to see this merged!

[benmccann]

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

[dominikg]

looks like they reverted it https://github.com/floating-ui/floating-ui/pull/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 https://github.com/floating-ui/floating-ui/pull/1526

[benmccann]

It looks like it's been fixed on the main branch for an eventual v3 https://github.com/floating-ui/floating-ui/pull/1502 and that https://github.com/floating-ui/floating-ui/pull/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]

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]

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]

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" } }

[skornel02]

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]

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

• the PR : https://github.com/bestguy/sveltestrap/pull/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]

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]

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 (https://github.com/floating-ui/floating-ui/pull/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]

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]

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

[godmar]

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]

@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]

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]

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

[benmccann]

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

[bestguy]

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

[benmccann]

Hurray! Thank you so much!

[laxadev]

@bestguy I added one more PR #564