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.

Sign up for GitHub

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

Jump to bottom

Use native import maps in development mode #14743

Merged
merged 1 commit into from
Oct 14, 2023
Merged

Use native import maps in development mode #14743

merged 1 commit into from
Oct 14, 2023

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Apr 2, 2022 •
edited
Loading

This patch seem to work fine locally now, and mozregression points to it being fixed in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1803984 which landed in Firefox 116.

By using the native import maps functionality, we can remove a development dependency. Please find the specification at https://wicg.github.io/import-maps/

@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 2 times, most recently from 82bca5d to eb34a71 Compare April 24, 2022 10:09
@Snuffleupagus Snuffleupagus mentioned this pull request Apr 30, 2022
Merged
@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 3 times, most recently from e49adda to 2ae674f Compare May 7, 2022 10:04
@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented May 7, 2022 •
edited
Loading

Note that https://bugzilla.mozilla.org/show_bug.cgi?id=1688879 has now landed in Firefox, however:

  • The functionality is disabled by default, even in Nightly; see dom.importMaps.enabled in about:config.
  • When manually enabled, the development viewer fails to load intermittently and the unit/font-tests fail consistently; this problem is being tracked in https://bugzilla.mozilla.org/show_bug.cgi?id=1762595 (at least I think so).

@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 2 times, most recently from a33036e to 2e9d3e2 Compare May 13, 2022 07:47
@calixteman
Copy link
Contributor

I'd like to have the possibility to test the import maps functionality.
Would it be possible to have a special target in gulpfile (e.g. gulp server-experimental or something like that) in order to test that stuff and potentially help to find some bugs ?

@Snuffleupagus
Copy link
Collaborator Author

I'd like to have the possibility to test the import maps functionality.

Just in case it's not immediately clear: Please note that we're already using import maps in development mode, since quite some time, thanks to the es-module-shims polyfill.

The point of this PR is only to remove that polyfill, and start using the native browser functionality. As mentioned in #14743 (comment), moving forward with this PR is currently blocked on bug 1762595 being fixed.

Would it be possible to have a special target in gulpfile (e.g. gulp server-experimental or something like that) in order to test that stuff and potentially help to find some bugs ?

Not easily, I don't think, and as mentioned native import maps currently don't work (properly) in Firefox anyway.

@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 2 times, most recently from 94beaa4 to 63c7e71 Compare May 29, 2022 12:58
@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 2 times, most recently from 370ffcf to e5150ec Compare June 10, 2022 20:21
@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 2 times, most recently from c4f09f8 to 66af544 Compare June 28, 2022 13:42
@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 2 times, most recently from 1644cbd to ba16eb8 Compare July 10, 2022 14:15
@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 2 times, most recently from 38012e5 to 1def54c Compare July 21, 2022 08:26
@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 2 times, most recently from 92b9d70 to 63eee31 Compare July 31, 2022 14:34
@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 2 times, most recently from 99619d6 to 2217075 Compare November 15, 2022 11:20
@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 4 times, most recently from 5b8be99 to 540f001 Compare June 14, 2023 15:27
@Snuffleupagus Snuffleupagus force-pushed the native-import-maps branch 3 times, most recently from 45b891e to 65639c3 Compare October 11, 2023 12:22
@Snuffleupagus Snuffleupagus changed the title [WIP] Use native import maps in development mode Use native import maps in development mode Oct 11, 2023
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/80be2ec620ec3e4/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/f82baadd10f845a/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/f82baadd10f845a/output.txt

Total script time: 24.62 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 16
  different first/second rendering: 3

Image differences available at: http://54.241.84.105:8877/f82baadd10f845a/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/80be2ec620ec3e4/output.txt

Total script time: 36.49 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  errors: 1
  different ref/snapshot: 17

Image differences available at: http://54.193.163.58:8877/80be2ec620ec3e4/reftest-analyzer.html#web=eq.log

@Snuffleupagus Snuffleupagus marked this pull request as ready for review October 11, 2023 13:05
@Snuffleupagus
Use native import maps in development mode
5e986cb 
This patch seem to work fine locally now, and `mozregression` points to it being fixed in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1803984 which landed in Firefox 116.

By using the native `import maps` functionality, we can remove a development dependency. Please find the specification at https://wicg.github.io/import-maps/
@timvandermeij timvandermeij merged commit 57866cd into mozilla:master Oct 14, 2023
@timvandermeij
Copy link
Contributor

Nice work; good to see that the upstream issues have been fixed and we can finally get rid of that extra dependency!

@Snuffleupagus Snuffleupagus deleted the native-import-maps branch October 14, 2023 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timvandermeij timvandermeij timvandermeij approved these changes

Assignees
No one assigned
Labels
test viewer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Snuffleupagus @calixteman @moz-tools-bot @timvandermeij