Add transpiled ES Module build with microbundle #9

romanzoller · 2019-01-25T14:57:42Z

Add module entrypoint (index.js)
Use whitelist of files (/src, /dist/sources.*) instead of .npmignore
Add build target using microbundle and rename existing example builds

- Add module entrypoint (index.js) - Use whitelist of files (/src, /dist/sources.*) instead of .npmignore - Add build target using microbundle and rename existing example builds See geoblocks/base#4

sbrunner · 2019-01-25T16:36:51Z

.gitignore

@@ -1,4 +1,5 @@
 /node_modules/
 /package-lock.json
 /dist/api
-/dist/main.js
+/dist/main.*
+/dist/sources.*


Why not just /dist/?

Because dist/index.html is (and was already before) part of the git source, it's the example entry point, so it should not be ignored. Ideally it should not be in dist IMHO, I can change that in a follow-up PR if you agree.

sbrunner · 2019-01-25T16:37:25Z

package.json

    "doc": "typedoc --out dist/api --theme minimal --readme none --hideGenerator --listInvalidSymbolLinks --toc none"
  },
+  "files": [
+    "/src",
+    "/dist/sources.*"


Why not just /dist?

Same here, /dist/index.html and /dist/main.js[.map] are example / example build files that don't need to be included in the npm package.

gberaudo · 2019-01-25T16:52:41Z

package.json

    "start": "webpack-dev-server --mode development --content-base dist/ --watch",
    "typecheck": "tsc --pretty",
    "lint": "npm run eslint && npm run typecheck",
-    "test": "npm run lint && npm run build && npm run build-debug",
+    "test": "npm run lint && npm run build-example && npm run build-example-debug",


In order for the microbundle build to be tested by travis, you should also add it here, right?

Yes, good point, I added it.

package.json

See https://webpack.js.org/configuration/resolve/#resolve-symlinks

gberaudo · 2019-02-04T12:40:58Z

@romanzoller, I pushed a commit to make "npm link" work.

gberaudo · 2019-02-04T12:42:14Z

@romanzoller, the code is still importing proj directly from the src files. You need to change that tp benefit from your changes in @geoblocks/proj.

So I did:

diff --git a/src/Swisstopo.js b/src/Swisstopo.js
index 9240e56..f9f8a37 100644
--- a/src/Swisstopo.js
+++ b/src/Swisstopo.js
@@ -1,7 +1,7 @@
 import olSourceWMTS from 'ol/source/WMTS.js';
 import olTilegridWMTS from 'ol/tilegrid/WMTS.js';
-import EPSG_2056 from '@geoblocks/proj/src/EPSG_2056.js';
-import EPSG_21781 from '@geoblocks/proj/src/EPSG_21781.js';
+import {EPSG_2056} from '@geoblocks/proj';
+import {EPSG_21781} from '@geoblocks/proj';
 
 
 /**

Then ran npm run start but it failed.

I think the reason is that:

I am using npm link;
I did not build the proj repo;
webpack is resolving the import to @geoblocks/proj/dist/proj.js due to the module entry and it does not exist.

If I build the proj project I guess it will succeed but then updating a proj file will not live reload the code anymore. Do you see a solution that keeps npm link working?

romanzoller · 2019-02-04T12:55:39Z

the code is still importing proj directly from the src files. You need to change that tp benefit from your changes in @geoblocks/proj.

This should not be required, I was trying to make only minimal changes in order to get things merged asap. The changes in @geoblocks/proj are only relevant for using proj somewhere else, we should be able to keep importing source files here.

See also https://github.com/camptocamp/ti-oerebviewer/pull/67/commits/f48961ac480a766272d4ae0164c37119a983e2d0 about npm link, I think the easiest way to get around the npm link mess would be to publish new versions of geoblocks/proj and geoblocks/sources.

gberaudo · 2019-02-04T13:33:30Z

@romanzoller, you are using the source here: https://github.com/camptocamp/ti-oerebviewer/pull/67/files#diff-ef47f283833c2e4c42778c3038ed1278L6. Since it depends on the non-transpiled ES6 sources from proj it will not fix your IE11 issue, right?

gberaudo · 2019-02-04T13:53:06Z

@romanzoller, I think having paths pointing to dist in the package.json we commit to github is wrong. We should (in @geoblocks/proj):

use module: src/index.js in the commited package.json;
modify it to module: dist/proj.js in the published package.json.

gberaudo · 2019-02-05T10:38:19Z

@romanzoller, @sbrunner, @fredj, would you be interested in a small (?) meeting this afternoon to:

discuss the status of Roman's PRs;
what works, what does not work yet;
take the decisions/compromise we need to accomodate all use cases (Ngeo, Ng-cli, geoblock, ...)

romanzoller · 2019-02-05T10:49:22Z

Since it depends on the non-transpiled ES6 sources from proj it will not fix your IE11 issue, right?

You are technically right, but I was lucky, because in geoblocks/sources, only 2 strings are imported from geoblocks/proj, so there is nothing that breaks IE11. I guess I should change it anyway.

I think having paths pointing to dist in the package.json we commit to github is wrong. We should (in @geoblocks/proj):

use module: src/index.js in the commited package.json;

modify it to module: dist/proj.js in the published package.json.

Pointing to dist for files that are built and included in the package published on npm is perfectly normal IMHO.

Unfortunately the meaning of "module" is only documented in a proposal and not standardized (yet), but I think your problem would be solved best by setting "modules.root": "src" as described here.

would you be interested in a small (?) meeting this afternoon

Sure!

gberaudo · 2019-02-07T08:55:46Z

@romanzoller, you need to release a geoblocks/proj package on npm and test it here.
You also need to change the imports here:

import EPSG_2056 from '@geoblocks/proj/src/EPSG_2056.js';
import EPSG_21781 from '@geoblocks/proj/src/EPSG_21781.js';

Please also add the "geoblocks_src: src/index.js" entry in package.json and I think it will be ready for merging.

gberaudo · 2019-02-07T08:59:27Z

Since it depends on the non-transpiled ES6 sources from proj it will not fix your IE11 issue, right?

You are technically right, but I was lucky, because in geoblocks/sources, only 2 strings are imported > from geoblocks/proj, so there is nothing that breaks IE11. I guess I should change it anyway.

I think it was even more luck as the registration of the projection is done as a side effect of the import.
It was luck that this code does not use (yet! ;)) some ES6 sugar.

romanzoller · 2019-02-07T16:38:41Z

you need to release a geoblocks/proj package on npm and test it here.

done 👍

Please also add the "geoblocks_src: src/index.js" entry in package.json and I think it will be ready for merging.

@gberaudo I don't really understand this part... Would you then remove the "source" entry? Is it causing any problems? And what is the purpose of adding "geoblocks_src"?

gberaudo · 2019-02-07T17:02:04Z

@romanzoller,

we should remove the source entry because people does not understand it is non standard;
we should add a "geoblocks_src" entry so that people wanting to transpile themselves can point to the sources.

romanzoller · 2019-02-07T17:09:00Z

Well, it's not standard, but it's used by microbundle, parcel, and maybe others, and might be standard in the future, whereas "geoblocks_src" is used by nobody and will for sure never be a standard, and (AFAIK) is also not necessary if you want to include sources in another project, you could already do that before without any "source" or "geoblocks_src" field and you can still do it now that "source" is there.

gberaudo · 2019-02-08T08:15:49Z

@romanzoller, I checked parcel and found it has 3 ways of using the source entry: https://en.parceljs.org/transforms.html#third-party-modules
Microbundle only recognize one, when the value is a path to a single file.
It makes even clearer my point that if something is not standard, relying on it will bring issues.

That is why I ask we use a custom entry for the geoblocks project that no one else will use inadvertently, and which semantics we can define clearly: geoblocks_src points to a single file entry point of standard Ecmascript (no esnext, no proposal, only the rock solid standard).

Regarding your second point:

if you want to include sources in another project, you could already do that before without any "source" or "geoblocks_src" field

In an application A, how would you depend on source version of the source and proj geoblocks without those fields? My understanding is that even if you import @geoblocks/source/src/Swisstopo.js that block depends on the transpiled @geoblocks/proj so it would not work like you expect.

romanzoller · 2019-02-08T08:53:03Z

In an application A, how would you depend on source version of the source and proj geoblocks without those fields? My understanding is that even if you import @geoblocks/source/src/Swisstopo.js that block depends on the transpiled @geoblocks/proj so it would not work like you expect.

Do you mean something like this? (tested and works in an empty temp folder, no "geoblocks_src" in the latest @geoblocks/proj)

npm i @geoblocks/proj esm proj4 ol
node -r esm
> import EPSG_2056 from '@geoblocks/proj/src/EPSG_2056';
undefined
> EPSG_2056
'EPSG:2056'

gberaudo · 2019-02-08T09:05:07Z

No, I am saying:

A

import Swisstopo from '@geoblocks/source/src/Swisstopo.js';
import EPSG_2056 from '@geoblocks/proj/src/EPSG_2056'.js;

My understanding is that in this snippet the @geoblocks/proj code is included 2 times:

as a transpiled dependency of @geoblocks/source (since source depends on the microbundle version of proj);
as a source dependency (since we directly import it from A).

The only way to avoid this is to have weback use entries like `geoblocks_src'.

No need to change anything with microbundle, it turns out that it already looks for ./src/index.js or ./index.js (in that order) by default.

romanzoller · 2019-02-08T13:55:55Z

Ok, thank you for the explanations @gberaudo, I added a commit => good to merge?

Add transpiled ES Module build with microbundle

811c747

- Add module entrypoint (index.js) - Use whitelist of files (/src, /dist/sources.*) instead of .npmignore - Add build target using microbundle and rename existing example builds See geoblocks/base#4

romanzoller mentioned this pull request Jan 25, 2019

Provide transpiled ES Module builds for ES5 targets such as Internet Explorer 11 geoblocks/base#4

Open

sbrunner reviewed Jan 25, 2019

View reviewed changes

gberaudo reviewed Jan 25, 2019

View reviewed changes

package.json Show resolved Hide resolved

Roman Zoller and others added 2 commits February 4, 2019 10:41

Run build target in "npm test"

7f4f729

Add support for npm link

2ac133d

See https://webpack.js.org/configuration/resolve/#resolve-symlinks

Update and use bundled version of @geoblocks/proj

120c6a5

Use "geoblocks_src" instead of "source"

68a75cf

No need to change anything with microbundle, it turns out that it already looks for ./src/index.js or ./index.js (in that order) by default.

gberaudo approved these changes Feb 8, 2019

View reviewed changes

romanzoller merged commit 8af650d into geoblocks:master Feb 8, 2019

romanzoller deleted the transpiled-build branch February 8, 2019 14:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add transpiled ES Module build with microbundle #9

Add transpiled ES Module build with microbundle #9

romanzoller commented Jan 25, 2019

sbrunner Jan 25, 2019

romanzoller Feb 4, 2019 •

edited

Loading

sbrunner Feb 4, 2019

sbrunner Jan 25, 2019

romanzoller Feb 4, 2019

gberaudo Jan 25, 2019

romanzoller Feb 4, 2019 •

edited

Loading

gberaudo commented Feb 4, 2019

gberaudo commented Feb 4, 2019 •

edited

Loading

romanzoller commented Feb 4, 2019

gberaudo commented Feb 4, 2019

gberaudo commented Feb 4, 2019

gberaudo commented Feb 5, 2019

romanzoller commented Feb 5, 2019

gberaudo commented Feb 7, 2019

gberaudo commented Feb 7, 2019

romanzoller commented Feb 7, 2019 •

edited

Loading

gberaudo commented Feb 7, 2019

romanzoller commented Feb 7, 2019

gberaudo commented Feb 8, 2019 •

edited

Loading

romanzoller commented Feb 8, 2019 •

edited

Loading

gberaudo commented Feb 8, 2019

romanzoller commented Feb 8, 2019

Add transpiled ES Module build with microbundle #9

Add transpiled ES Module build with microbundle #9

Conversation

romanzoller commented Jan 25, 2019

sbrunner Jan 25, 2019

Choose a reason for hiding this comment

romanzoller Feb 4, 2019 • edited Loading

Choose a reason for hiding this comment

sbrunner Feb 4, 2019

Choose a reason for hiding this comment

sbrunner Jan 25, 2019

Choose a reason for hiding this comment

romanzoller Feb 4, 2019

Choose a reason for hiding this comment

gberaudo Jan 25, 2019

Choose a reason for hiding this comment

romanzoller Feb 4, 2019 • edited Loading

Choose a reason for hiding this comment

gberaudo commented Feb 4, 2019

gberaudo commented Feb 4, 2019 • edited Loading

romanzoller commented Feb 4, 2019

gberaudo commented Feb 4, 2019

gberaudo commented Feb 4, 2019

gberaudo commented Feb 5, 2019

romanzoller commented Feb 5, 2019

gberaudo commented Feb 7, 2019

gberaudo commented Feb 7, 2019

romanzoller commented Feb 7, 2019 • edited Loading

gberaudo commented Feb 7, 2019

romanzoller commented Feb 7, 2019

gberaudo commented Feb 8, 2019 • edited Loading

romanzoller commented Feb 8, 2019 • edited Loading

gberaudo commented Feb 8, 2019

romanzoller commented Feb 8, 2019

romanzoller Feb 4, 2019 •

edited

Loading

romanzoller Feb 4, 2019 •

edited

Loading

gberaudo commented Feb 4, 2019 •

edited

Loading

romanzoller commented Feb 7, 2019 •

edited

Loading

gberaudo commented Feb 8, 2019 •

edited

Loading

romanzoller commented Feb 8, 2019 •

edited

Loading