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

Added support for pswpModule import without absolute path #1764

Merged
merged 2 commits into from
Jun 24, 2021
Merged

Added support for pswpModule import without absolute path #1764

merged 2 commits into from
Jun 24, 2021

Conversation

brunogrcsada
Copy link
Contributor

This pull request gives the ability for users to import the pswpModule script without an absolute path; it's extremely useful when PhotoSwipe is saved as a node module. However, it still allows for the absolute path to be given instead...

Example:

import PhotoSwipeLightbox from 'photoswipe/dist/photoswipe-lightbox.esm.js';
import {default as PhotoMain} from 'photoswipe/dist/photoswipe.esm.js';
	
import 'photoswipe/dist/photoswipe.css';

let lightbox;

onMount(() => {
        lightbox = new PhotoSwipeLightbox({
	        gallerySelector: '#gallery--simple',
		childSelector: 'a',
		pswpModule: PhotoMain
	});
        lightbox.init();
});

@dimsemenov
Copy link
Owner

Thank you. Can you please modify the pull request so it changes source files (from src/ folder) and not distribution (dist/), specifically:

@brunogrcsada
Copy link
Contributor Author

No worries :), done and dusted, everything should be good to go.

@dimsemenov dimsemenov merged commit b669a09 into dimsemenov:v5-beta Jun 24, 2021
@dimsemenov
Copy link
Owner

dimsemenov commented Jun 24, 2021

thanks, should now be working in 5.0.3

import PhotoSwipeLightbox from 'photoswipe/dist/photoswipe-lightbox.esm.js';
import PhotoSwipe from 'photoswipe/dist/photoswipe.esm.js';

import 'photoswipe/dist/photoswipe.css';

const options = {
  gallerySelector: '#my-gallery',
  childSelector: 'a',
  pswpModule: PhotoSwipe
};
const lightbox = new PhotoSwipeLightbox(options);
lightbox.init();
npm install --save git://github.com/dimsemenov/photoswipe#v5-beta

@dangelion
Copy link

dangelion commented Jun 25, 2021

@brunogrcsada @dimsemenov Yes, this works! 🎉 Even if at the build there's this warning

WARNING in ./node_modules/photoswipe/dist/photoswipe-lightbox.esm.js 230:38-52
Critical dependency: the request of a dependency is an expression

webpack compiled with 1 warning

Any way to solve it?

@brunogrcsada
Copy link
Contributor Author

brunogrcsada commented Jun 25, 2021

@dangelion I managed to get the warning myself with a Next.js project. I was using Svelte and it's a breeze, what JS library are you using? The reason for webpack not liking this is because it prefers explicit paths when importing a module; this isn't a big issue but I'll look into it.

Adding webpackIgnore to the import fixes this issue:

return typeof module === 'string' ? import(/* webpackIgnore: true */ module) : module;

I'll push this in a second.

@brunogrcsada
Copy link
Contributor Author

I've added the suppress comment to my latest pull request :) @dimsemenov @dangelion

@dangelion
Copy link

Hi @brunogrcsada thanks, I can't see your latest pr. How can get it?

@brunogrcsada
Copy link
Contributor Author

@dangelion Here it is, I also added image caption support: #1766

@dangelion
Copy link

dangelion commented Jun 28, 2021

Thanks @brunogrcsada now I see it 4944ac8

@dimsemenov When you think you'll release it?

@dangelion
Copy link

dangelion commented Jul 30, 2021

@dimsemenov Is this solved in the new 5.1.0?

@acwolff
Copy link

acwolff commented Jul 30, 2021

@ dangelion I don't know whether this answers your question, but I use version 5.1.0 with only relative paths:

   	import PhotoSwipeLightbox from './res/photoswipe-lightbox.esm.js';
	import PhotoSwipeDynamicCaption from './res/photoswipe-dynamic-caption-plugin.esm.js';
	const options = {
	  gallerySelector: '#gallery',
	  childSelector: 'a',
 	  pswpModule: './photoswipe.esm.js'
 	};

This works fine!

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.

None yet

4 participants