[MERGE] *: Add support for native JS modules and sourcemaps #63177

SimonGenin · 2020-12-10T14:39:58Z

Because of the way Odoo works at its core, we do not know before hand
which files will be loaded as an asset in the browser, because it
depends on the installed Odoo addons. This is why it is historically
difficult to integrate Odoo with standard JS tooling, and this is why
Odoo needs to use a custom javascript module system.

However, there is a way to use native JS modules (and gain all the
benefits from it: IDE autocompletion, ease of refactoring, intellisense,
...): we can write JS as native JS modules, but convert them at runtime
into Odoo custom modules. This is exactly the strategy applied by this
PR.

This has a lot of benefits, but there is a downside: we can no longer
serve statically JS files in debug=assets. This would be a dealbreaker,
if we did not have sourcemaps.

So, this PR introduces

support for native JS modules
add a specific asset controller /web/assets/
generate sourcemaps (for debug=assets mode)

Note that part of the complexity of this work is cause by the debug=assets mode, where we need to have a fixed route for the debug asset bundle (so we do not lose breakpoints when debugging), but we still need to safely regenerate the bundles whenever the JS code is modified.

robodoo · 2020-12-10T14:40:14Z

SimonGenin · 2020-12-14T10:33:18Z

@FrancoisGe

odoo/addons/base/models/assetsbundle.py

odoo/addons/base/transpiler/source_map_generator.py

odoo/addons/base/transpiler/utils/__init__.py

odoo/addons/base/transpiler/utils/utils.py

odoo/addons/test_transpiler/test_transpiler.py

odoo/http.py

odoo/addons/base/transpiler/utils/base64VLQ.py

odoo/addons/base/transpiler/utils/__init__.py

odoo/addons/base/transpiler/utils/utils.py

odoo/addons/base/models/assetsbundle.py

odoo/addons/base/transpiler/codes/alias.js

odoo/addons/base/transpiler/sourcemap_generator.py

odoo/addons/base/models/assetsbundle.py

ged-odoo · 2021-02-15T08:01:00Z

robodoo r+

robodoo · 2021-02-15T08:01:11Z

@ged-odoo, you may want to rebuild or fix this PR as it has failed CI.

ged-odoo · 2021-02-15T08:16:08Z

robodoo merge

robodoo · 2021-02-15T08:16:20Z

Merge method set to merge directly, using the PR as merge commit message

Xavier-Do · 2021-02-15T08:25:58Z

@robodoo override=ci/style

"id" was already used before and a lot of line should be adapted + potential route change (?id=)
questionnable rule for controllers

odony · 2021-02-15T08:27:49Z

addons/web/controllers/main.py

-        '/web/content/<int:id>/<string:filename>',
-        '/web/content/<int:id>-<string:unique>',
-        '/web/content/<int:id>-<string:unique>/<string:filename>',
-        '/web/content/<int:id>-<string:unique>/<path:extra>/<string:filename>',


any reason for dropping these alternative routes? In particular, the /web/content/<int:id>/<string:filename> looks like it may be useful outside of the context of assets, no?

only the desire to simplify as much as possible. Do you want to add them back?

odony · 2021-02-15T08:45:36Z

addons/web/controllers/main.py

+    def content_assets(self, xmlid=None, model='ir.attachment', id=None, field='datas',
+                       filename=None, filename_field='name', unique=None, mimetype=None,
+                       download=None, data=None, token=None, access_token=None, **kw):
+


Here, on the other hand, I would only accept the parameters that we know we'll be using. e.g. not xmlid, model, field, filename_field, token, access_token, mimetype, download, ...

Since this is a new route dedicated to assets there's no legacy to support for other URL patterns. If we accept more parameters now, it may make it harder to implement more asset-specific logic in the future.

It will also avoid potential security-related issues related to model, token, etc. in the future, as we don't want to open more doors here that strictly necessary.

**kw is kept to support extra params like debug=xxx without crashing, but we ignore query string params we don't need to accept.

Suggested change

def content_assets(self, xmlid=None, model='ir.attachment', id=None, field='datas',

filename=None, filename_field='name', unique=None, mimetype=None,

download=None, data=None, token=None, access_token=None, **kw):

def content_assets(self, id=None, filename=None, unique=None, extra=None, **kw):

good time to make extra a named parameter too, no?

robodoo · 2021-02-15T08:50:16Z

Linked pull request(s) odoo/enterprise#16181, odoo/upgrade#2154 not ready. Linked PRs are not staged until all of them are ready.

Because of the way Odoo works at its core, we do not know before hand which files will be loaded as an asset in the browser, because it depends on the installed Odoo addons. This is why it is historically difficult to integrate Odoo with standard JS tooling, and this is why Odoo needs to use a custom javascript module system. However, there is a way to use native JS modules (and gain all the benefits from it: IDE autocompletion, ease of refactoring, intellisense, ...): we can write JS as native JS modules, but convert them at runtime into Odoo custom modules. This is exactly the strategy applied by this PR. This has a lot of benefits, but there is a downside: we can no longer serve statically JS files in debug=assets. This would be a dealbreaker, if we did not have sourcemaps (implemented in the next commit). This commit introduces the python code that will transpile native JS modules into odoo JS modules. Task ID: 2414902 PR: 63177 Co-authored-by: Francois (fge) <fge@odoo.com>

Because of the previous commit introducing support for native JS modules, it is no longer possible to serve statically JS files in debug=assets mode. Because of that, we serve a (non minified) bundle instead, but with sourcemaps. This commit contains the code for the sourcemap generator. Part of PR 63177 Co-authored-by: Simon Genin (ges) <ges@odoo.com>

@web

With the new native JS module system, we have a lot of new features for the developer: autocompletion, docstrings, ... However, it does not work across modules: if a JS file in /addons/stock/static/src/some_file.js want to import a file in web, say /addons/web/static/src/blabla.js, we will need to use a statement like this: import { something } from '@web/blabla'; Obviously, there is no automatic way for IDEs to know that '@web' should map to 'addons/web'. This is why we propose to use a tsconfig.json that defines the mapping between modules and their paths. This is not mandatory, and only affects those developers that work commonly in JS. Part of PR 63177 Co-authored-by: Francois (fge) <fge@odoo.com>

The previous work on adding support for native JS modules needs to adapt some existing files, which have an incompatible name (with a '/'). Also, we convert a few JS file in /web to the native JS module system, to show that it can work. Part of PR 63177 Co-authored-by: Simon Genin (ges) <ges@odoo.com>

ged-odoo · 2021-02-15T09:30:36Z

robodoo r+

Part of PR 63177

ged-odoo · 2021-02-15T10:00:27Z

robodoo r+

randall-vx · 2021-02-15T14:38:06Z

@oscarolar

SimonGenin force-pushed the master-assets-for-wowl-ges-fge branch 2 times, most recently from 368cd39 to cbae023 Compare December 15, 2020 13:41

FrancoisGe force-pushed the master-assets-for-wowl-ges-fge branch from ca4b8b7 to e4f160c Compare December 16, 2020 08:45

C3POdoo added the RD research & development, internal work label Dec 18, 2020

FrancoisGe force-pushed the master-assets-for-wowl-ges-fge branch from b04ac31 to db1b1c7 Compare January 6, 2021 14:31

SimonGenin force-pushed the master-assets-for-wowl-ges-fge branch from 900607e to 8e65995 Compare January 7, 2021 15:56

SimonGenin changed the title ~~Work on assets for wowl deployement~~ Introduction of odoo modules (ES6) and new debug asset mode Jan 7, 2021

FrancoisGe force-pushed the master-assets-for-wowl-ges-fge branch from a03ad34 to 4f35b61 Compare January 8, 2021 09:20

SimonGenin requested a review from a team as a code owner January 8, 2021 12:03

aab-odoo force-pushed the master-assets-for-wowl-ges-fge branch from 885cfda to 4e3c94b Compare January 11, 2021 08:43

SimonGenin force-pushed the master-assets-for-wowl-ges-fge branch 2 times, most recently from 08713d5 to e81ef04 Compare January 19, 2021 13:18

SimonGenin requested a review from a team as a code owner January 20, 2021 10:51

FrancoisGe reviewed Jan 22, 2021

View reviewed changes