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

fix: Use existing importmap for build by passing --map flag #2555

Merged
merged 4 commits into from
Dec 9, 2023

Conversation

JayaKrishnaNamburu
Copy link
Member

Adds --map option for jspm build command.
While moving the jspm-vscode project to use jspm-cli workflow. We will generate the importmap.json only once. And use it for re-runs or update it only when needed.

So, when the extension is being built. We should use the existing map. Currently the cli throws an error because --map is not allowed for build command.

Usage reference -> https://github.com/jspm/jspm-vscode/blob/refactor-build-workflow/chompfile.toml#L9

Eg

jspm build --map importmap.json --config rollup-config.mjs

@JayaKrishnaNamburu JayaKrishnaNamburu self-assigned this Dec 7, 2023
@JayaKrishnaNamburu JayaKrishnaNamburu changed the title fix: Allow --map to be passed while using jspm build fix: Use existing importmap for build by passing --map flag Dec 7, 2023
@guybedford
Copy link
Member

Looks good thanks. Any idea about the CI failure? Note that most CLI operations should automatically pick up the local importmap.json file without it needing to be provided via an explicit flag. Can we not do the same for the build?

@JayaKrishnaNamburu
Copy link
Member Author

Yes, the cli is picking the local build file by default. Needed only if we change the file name to something else than importmap.json
https://github.com/jspm/jspm-cli/blob/main/src/utils.ts#L213-L222

@JayaKrishnaNamburu
Copy link
Member Author

The CI failures are there for a while, should look into it. Something with assertions

@JayaKrishnaNamburu
Copy link
Member Author

Fixed tests for one use-case. There is another use-case that is failing for jspm install lit from esm.sh now. But i have a hunch it might not be realted to the generator.

Here is the error

Error: No './css-tag' exports subpath defined in https://esm.sh/*@lit/[email protected]/ resolving @lit/reactive-element/css-tag imported from https://esm.sh/v135/@lit/[email protected]/X-ZS8q/esnext/reactive-element.mjs.

Error: Scenario "jspm install lit -p esm.sh -e production" failed.
    at runScenario (file:///home/runner/work/jspm-cli/jspm-cli/test/scenarios.ts:1:845)
    at async runScenarios (file:///home/runner/work/jspm-cli/jspm-cli/test/scenarios.ts:1:444) {
  [cause]: AssertionError [ERR_ASSERTION]: undefined == true
      at Object.validationFn (file:///home/runner/work/jspm-cli/jspm-cli/test/providers.test.ts:1:1140)
      at runScenario (file:///home/runner/work/jspm-cli/jspm-cli/test/scenarios.ts:1:790)
      at async runScenarios (file:///home/runner/work/jspm-cli/jspm-cli/test/scenarios.ts:1:444) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: undefined,
    expected: true,
    operator: '=='
  }
}
Unable to complete all tasks.
x test/providers.test.ts [24.707770[31](https://github.com/jspm/jspm-cli/actions/runs/7144479899/job/19458208881#step:7:32)s]
x :test:##
x :test

The package @lit/reactive-element seems to create a export map with .js extension. And maybe the us causing the issue when trying to access the same without extensions ?
https://unpkg.com/browse/@lit/[email protected]/package.json

@guybedford
Copy link
Member

Can we at least disable the test and post a new issue to reenable it? We can't land PRs if CI is failing...

Otherwise looks good to land and thanks for clarifying importmap.json is already read by default.

@JayaKrishnaNamburu
Copy link
Member Author

Disabled esm.sh provider and loged a bug with repro on the generator.
jspm/generator#335

@JayaKrishnaNamburu JayaKrishnaNamburu merged commit 4bd0867 into main Dec 9, 2023
4 checks passed
@JayaKrishnaNamburu JayaKrishnaNamburu deleted the fix/add-map-config-for-build branch December 9, 2023 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants