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

chore: added plugin for automatic importmap generation #393

Merged
merged 2 commits into from
Jul 21, 2023

Conversation

Fredx87
Copy link
Contributor

@Fredx87 Fredx87 commented Jul 20, 2023

Description

This PR improves the management of the import map, which is now generated automatically from the chunks file emitted by rollup, instead of manually doing it.

I added a plugin that runs after Rollup generates the bundle and:

  1. replaces relative imports with bare imports in the emitted files
  2. creates an import map and inject it in the document_head.hbs template, replacing the existing one.

Note: the first step is done with a basic string replace, e.g. './a.js' is replaced with 'a', and this should cover both static imports (import something from './a.js') and dynamic ones (import('./a.js')). Maybe there is a better way to do it, but I didn't manage to find one.

Screenshots

Checklist

  • 📗 all commit messages follow the conventional commits standard
  • ⬅️ changes are compatible with RTL direction
  • ♿ Changes to the UI are tested for accessibility and compliant with WCAG 2.1.
  • 📝 changes are tested in Chrome, Firefox, Safari and Edge
  • 📱 changes are responsive and tested in mobile
  • 👍 PR is approved by @zendesk/vikings

@Fredx87 Fredx87 requested a review from a team as a code owner July 20, 2023 15:55
@Fredx87 Fredx87 force-pushed the gianluca/import-map-plugin branch from 72bafc9 to 094130c Compare July 20, 2023 15:57
@Fredx87 Fredx87 force-pushed the gianluca/import-map-plugin branch from 094130c to 45e8542 Compare July 21, 2023 07:08
Copy link
Contributor

@luis-almeida luis-almeida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added a few minor comments but looks pretty good!

Before approving I'd just suggest to add *.mjs to be linted by eslint.
Also not sure why prettier is no longer working (maybe just for me?) but that can of course be addressed in a following PR. Never-mind, it was local issue so still works!

generate-import-map.mjs Outdated Show resolved Hide resolved
generate-import-map.mjs Outdated Show resolved Hide resolved

const newImportMap = `<script type="importmap">
${JSON.stringify(importMap, null, 2)}
</script>`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: to keep the code alignment [].join('\n') is also an option.

@Fredx87 Fredx87 merged commit adfdfda into v4-alpha Jul 21, 2023
2 checks passed
@Fredx87 Fredx87 deleted the gianluca/import-map-plugin branch July 21, 2023 12:20
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.

2 participants