Skip to content
This repository has been archived by the owner on Nov 9, 2024. It is now read-only.

ESM with Importmaps #990

Closed
coder2000 opened this issue Oct 24, 2021 · 14 comments
Closed

ESM with Importmaps #990

coder2000 opened this issue Oct 24, 2021 · 14 comments
Labels
🐛 bug 🚧 unconfirmed

Comments

@coder2000
Copy link

Bug description

I know importmaps aren't on most peoples horizon but it is coming up quick with Rails developers and the release of Rails 7. Currently tippy requires a build environment like webpack, rollup or esbuild however with importmaps you just include the final ESM build from a CDN so there is no build step.

When I try to use tippy in this fashion it fails with Uncaught ReferenceError: process is not defined.

Reproduction

As I can't include the ESM shims this pen will only work on Chromium based browsers. The error will be visible in the browser console not the codepen for some reason.

https://codepen.io/coder2000/pen/mdMRgLL
@atomiks
Copy link
Owner

atomiks commented Oct 29, 2021

Can you polyfill window.process = { env: { NODE_ENV: 'development' }} etc? This way you will continue to get useful warnings/error messages while developing.

@coder2000
Copy link
Author

Just tried it in my app and in the code pen. No change, unless I am doing it wrong.

@atomiks
Copy link
Owner

atomiks commented Oct 29, 2021

The polyfill will need to be added before all of your scripts; placing it in <head> in an inline script tag would work.

@coder2000
Copy link
Author

Ok, that works, https://coder2000.github.io/. Codepen it seems doesn't like to do importmaps.

@sedubois
Copy link

sedubois commented Feb 2, 2022

I know importmaps aren't on most peoples horizon but it is coming up quick with Rails developers and the release of Rails 7.

I'm one of those Rails 7 developers and hit this error process is not defined. Rather than polyfill, in my view a better fix would be to have a production-ready bundle where the dev code is stripped out for compactness. Would that make sense?

@gregblass
Copy link

gregblass commented Mar 22, 2022 •
edited
Loading

+1 for proper support/documentation here.

Rails 7 introduced importmaps by default, and I must say - it is pretty awesome to not have to deal with a build environment.

You may want to consider adding some information to the documentation on how one might use Tippy using importmaps, as more people may start adopting it as time goes on.

@dougc84
Copy link

dougc84 commented May 15, 2022

Seconded @gregblass - I just spent an hour trying to make tippy work on a new Rails 7 app only to end up here.

@ericraio
Copy link

same here for tippyjs

@mildred
Copy link

mildred commented Aug 19, 2022 •
edited
Loading

The polyfill, while it can make it work, is not really adequate. I'm coming here because tippy is a transitive dependency and for a library to require a global setting like this is not a good practice.

Could the code be updated to check for typeof(process) !== 'undefined' like explained in https://jspm.org/docs/cdn#universal-module-semantics ?

When accessing environment-specific globals like process in Node.js, always use a guard like typeof process !== 'undefined'as they won't necessarily be available in other environments. Ideally, rather import these modules where possible - import process from 'process'.

@atomiks
Copy link
Owner

atomiks commented Aug 20, 2022

Sorry I know this is annoying :\

Any type of guard like that always seemed to break the dead code elimination by bundlers when I checked (defeating the purpose of the check), which causes the warning messages to end up in production bundles inflating the size.

@woodardj
Copy link

Defining window.process in the browser to avoid a problem with node environment bundlers feels like a really non-solution, and could easily lead to issues with other libraries. @atomiks when you say it "inflates the size" of a production bundle, what are we talking? A couple of kb? I'd much rather have this work out of the box with a proper environment check as @mildred references than potentially save microseconds of load time.

@santiagodoldan
Copy link

It would be great to use this library via importmap out of the box, I agree that defining the process object seems too hacky IMHO

@BakiVernes
Copy link

Any updates on this? @atomiks would a PR on this be considered?

@Caleb-T-Owens Caleb-T-Owens mentioned this issue Jan 25, 2024
Open
@edolix
Copy link

edolix commented Mar 1, 2024

@atomiks can we help in any way? Following the last comment: would a PR on this be considered?

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
🐛 bug 🚧 unconfirmed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests