-
Notifications
You must be signed in to change notification settings - Fork 189
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
Use type: module for better ES module handling #267
Conversation
8b1559c
to
6145b24
Compare
When this is in, I can rework #253 to use |
@ahocevar Thanks for your effort! I see that including geotiff.js resulted in build errors for dependent libraries such as OpenLayers. As far as I understand the issue, using In principle I'm fine with the changes, but what does that mean for other dependent projects? Do they have to change their import syntax or built config? If yes, I suggest we aim for a major release, otherwise we could do a minor one. I'm a little out of the loop in regards to JS build systems. We switched several times already and all had their separate quirks and issues. The biggest one always seems to be related to how the workers are set up and their respective code is split into separate chunks when building. What do you consider the benefits of rollup or vite in comparison to what we currently have? |
The advantage of importing ES modules rather than what's in
There should be no need to change anything in existing projects.
My goal here is to remove exactly these quirks, i.e. to ship a package that can be used both standalone and as a dependency without having to deal with workers. Tools like rollup or vite (which also depends on rollup) can simply be better configured than parcel to achieve this. But I see this as a future effort, so other than #242, I did not include it in this pull request. |
Closing this due to potential issues with the |
When #268 is in, I think it would be worthwhile to take the minimal required changes from #242 to fix ESM exports, i.e. renaming the source files to |
2 questions: |
This branch was not merged. See #269 for a replacement. But the |
6.6.1 is the ol version I am using. When using a newer version we will get the resolve import error. |
Yes I will try to do that. |
@ahocevar For now we fixed this with an empty class for some files: check for node-resolve:empty.js in rollup load function |
This pull request makes it easier to use geotiff.js in modern JavaScript environments (ES module capable Node versions, modern bundlers). It is somewhat related to #242.
By importing from
'geotiff/src/geotiff.js'
instead ofgeotiff
, consumers can opt in to unconditionally using the native ES module code.See openlayers/openlayers#13114 (comment).